volatiletech / sqlboiler

Generate a Go ORM tailored to your database schema.
BSD 3-Clause "New" or "Revised" License
6.66k stars 539 forks source link

Upsert Problem on Multiple Columns Unique Key (MySQL) #328

Open ceshihao opened 6 years ago

ceshihao commented 6 years ago

Upsert() returns a error cannot upsert with a table that cannot conflict on a unique column.

What version of SQLBoiler are you using (sqlboiler --version)?

SQLBoiler v3.0.0-rc9

If this happened at runtime what code produced the issue? (if not applicable leave blank)

Yes

Please provide a relevant database schema so we can replicate your issue (Provided you are comfortable sharing this)

CREATE TABLE `test` (
  `id` int(11) unsigned NOT NULL AUTO_INCREMENT,
  `node_id` int(11) NOT NULL,
  `node_type` varchar(32) NOT NULL DEFAULT '',
  `parameter` varchar(32) DEFAULT NULL,
  PRIMARY KEY (`id`),
  UNIQUE KEY `node_id_type` (`node_id`,`node_type`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8;

Further information. What did you do, what did you expect?

Upsert() can work well with Multiple Columns Unique Key. After a quick investigation, I find the error is from

var mySQLTestUniqueColumns = []string{
    "id",
}

func (o *Test) Upsert(ctx context.Context, exec boil.ContextExecutor, updateColumns, insertColumns boil.Columns) error {
    ...
    nzUniques := queries.NonZeroDefaultSet(mySQLTestUniqueColumns, o)

    if len(nzUniques) == 0 {
        return errors.New("cannot upsert with a table that cannot conflict on a unique column")
    }
    ...
}
ceshihao commented 6 years ago

I find it is because sqlboiler-mysql driver does not think multiple columns unique key as unique.

https://github.com/volatiletech/sqlboiler/blob/84770d21d5a1eaa3ecf93cac0eced39b3c52d078/drivers/sqlboiler-mysql/driver/mysql.go#L195-L203

@aarondl Do you have any thought?

aarondl commented 6 years ago

Hi @ceshihao. This is actually a pretty big problem. The idea of uniqueness in sqlboiler is based on single columns but that's of course not the only possible unique constraint inside a database.

We may have to change the way we do uniques and instead make it a list of lists of columns.

I'm trying to understand how this would affect the rest of sqlboiler's code. Obviously mysql's upsert would change. Some of the relationship stuff looks for unique and that would change too.

I'll have to take a closer look and see what the tradeoffs are here. Worst case you should be able to fall back to raw queries here as a workaround temporarily.

It's on my radar. :)

ceshihao commented 6 years ago

After a quick investigation, I think mysql Upsert can still use LastInsertID().

The problem is mentioned in #177. If insert conflict and update the existing row to its current values (dummy update), LastInsertID() does not return the correct ID of the row.

Mentioned in https://dev.mysql.com/doc/refman/5.7/en/insert-on-duplicate.html

If a table contains an AUTO_INCREMENT column and INSERT ... ON DUPLICATE KEY UPDATE inserts or updates a row, the LAST_INSERT_ID() function returns the AUTO_INCREMENT value.

But I find a workaround to use a dummy column in update which can resolve the problem. Mentioned in https://dev.mysql.com/doc/refman/5.7/en/insert-on-duplicate.html too.

A way to make things work is to use a dummy column, so if you have a table with auto_increment column ID and unique key a,b and a smallint dummy column for instance, the query might look like this: INSERT INTO test (a,b) VALUES ('1','2') ON DUPLICATE KEY UPDATE ID=LAST_INSERT_ID(ID),Dummy = NOT dummy; Now, SELECT LAST_INSERT_ID(); will return the correct ID.

aarondl commented 6 years ago

If a=1 OR b=2 matches several rows, only one row is updated. In general, you should try to avoid using an ON DUPLICATE KEY UPDATE clause on tables with multiple unique indexes.

This is awkward. Thanks mysql :(

the affected-rows value per row is 1 if the row is inserted as a new row, 2 if an existing row is updated, and 0 if an existing row is set to its current values

This might be useful. Just noting it here.

The meat of this issue however is how sqlboiler views unique indexes. Currently it doesn't actually understand them correctly. It looks at a column individually and checks if it's unique. So the entire data model for it is wrong. Right now for our purposes it mostly works because the only real important thing about unique columns from sqlboiler's perspective is foreign keys. And since it rarely makes sense to have a composite unique index around a foreign key we don't run into problems other than this one. So it's not an easily fixable thing and because this is Upsert I'm not super keen on diving in and fixing this at the moment. But that's really what we'd probably need to do here, is start considering uniqueness as something separate from the column definition, and pipe that change through the software.

cad commented 5 years ago

I would be glad to see this fixed. In the meantime, an official workaround from the devs would also be appreciated.

aarondl commented 5 years ago

@cad: I don't really have a workaround to keep mysql Upsert working atm. Simply use a transaction and do a select and then an insert to deal with the problem.

nikooo777 commented 2 years ago

I am reviving this issue as it's still relevant. I find myself having to work around this issue quite often in my codebase. Has anything changed that would make solving this issue any easier?

stephenafamo commented 2 years ago

I don't think anything particular has changed. If you send in a PR for this, I would be happy to review/merge.

Just remember that we cannot break backwards compatibility, so instead of changing the Upsert method signature, you may need to add a second Upsert method.