Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
ceshihao opened this issue Jul 9, 2018 · 8 comments
Open

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

ceshihao opened this issue Jul 9, 2018 · 8 comments

Comments

@ceshihao
Copy link
Contributor

ceshihao commented Jul 9, 2018

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
Copy link
Contributor Author

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

exists (
select c.column_name
from information_schema.table_constraints tc
inner join information_schema.key_column_usage kcu
on tc.constraint_name = kcu.constraint_name and tc.table_name = kcu.table_name and tc.table_schema = kcu.table_schema
where c.column_name = kcu.column_name and tc.table_name = c.table_name and
(tc.constraint_type = 'PRIMARY KEY' or tc.constraint_type = 'UNIQUE') and
(select count(*) from information_schema.key_column_usage where table_schema = kcu.table_schema and table_name = tc.table_name and constraint_name = tc.constraint_name) = 1
) as is_unique

@aarondl
Do you have any thought?

@aarondl
Copy link
Member

aarondl commented Jul 12, 2018

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
Copy link
Contributor Author

ceshihao commented Jul 13, 2018

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 aarondl added the bug label Jul 15, 2018
@aarondl
Copy link
Member

aarondl commented Jul 15, 2018

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
Copy link

cad commented Nov 28, 2018

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

@aarondl
Copy link
Member

aarondl commented Dec 1, 2018

@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
Copy link

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
Copy link
Collaborator

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants