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

UPDATE with JOIN for MySQL dialect: Column in field list is ambiguous #344

Closed
devurandom opened this issue Aug 11, 2021 · 5 comments
Closed
Labels
bug It's broken, I'll fix it!
Milestone

Comments

@devurandom
Copy link

devurandom commented Aug 11, 2021

(sql/format {:update :subscriptions
             :join   [:customers [:= :customers/id :subscriptions/customer_id]]
             :set    {:subscriptions/deleted_at now}
             :where  [:and
                      [:= :customers/phone phone-number]
                      [:is :customers/deleted_at nil]
                      [:is :subscriptions/deleted_at nil]]}
            {:dialect :mysql})

Produces:

[UPDATE `subscriptions` INNER JOIN `customers` ON `customers`.`id` = `subscriptions`.`customer_id` SET `deleted_at` = ? WHERE (`customers`.`phone` = ?) AND (`customers`.`deleted_at` IS NULL) AND (`subscriptions`.`deleted_at` IS NULL) #object[java.time.LocalDateTime 0x656af92f 2021-08-11T08:49:46.814862] <<SOME_PHONE_NUMBER>>]

MySQL will complain:

Execution error (SQLIntegrityConstraintViolationException) at com.mysql.cj.jdbc.exceptions.SQLError/createSQLException (SQLError.java:117).
Column 'deleted_at' in field list is ambiguous

The reason is that deleted_at needs to be prefixed with subscriptions. When I use the following string instead, MySQL accepts it:

["UPDATE `subscriptions` INNER JOIN `customers` ON `customers`.`id` = `subscriptions`.`customer_id` SET `subscriptions`.`deleted_at` = ? WHERE (`customers`.`phone` = ?) AND (`customers`.`deleted_at` IS NULL) AND (`subscriptions`.`deleted_at` IS NULL)"
 now
 phone-number]

I already tried to enforce this in the code above by namespacing the keyword, but for some reason HoneySQL did not adopt that into the final string.

@seancorfield
Copy link
Owner

I'll have to do some research across multiple databases. This is currently the way it is because at least one database (maybe several) does not allow qualified names in SET clauses, as I recall.

@devurandom
Copy link
Author

I'll have to do some research across multiple databases. This is currently the way it is because at least one database (maybe several) does not allow qualified names in SET clauses, as I recall.

Would it be possible to use qualified names just for MySQL for now? My tests show that MySQL 8 supports this.

@seancorfield
Copy link
Owner

As I said, I will do some research across multiple databases and decide on the best solution for this problem because HoneySQL must support a wide variety of databases. My baseline for MySQL support is 5.7. I need to figure out how changing the behavior of SET would factor into the existing dialect support system. I will probably take a look at this over the weekend, which is likely to be my next window to work on OSS projects.

@seancorfield
Copy link
Owner

OK, I'm starting to look into this now and came across #317 which is why qualified keywords have their namespace portion dropped in SET statements: PostgreSQL is a lot more popular with Clojurians than MySQL and is generally closer to "Standard SQL" and essentially the default mode in HoneySQL.

Then I looked through the tests, figuring I must have a test for #317 and indeed I do: https://github.com/seancorfield/honeysql/blob/develop/test/honey/sql_test.cljc#L353-L375

Interestingly, explicitly dotted names are respected so you could use :subscriptions.deleted_at and you would get what you want.

However, I've gone ahead and modified how qualifiers work in SET just for MySQL so your :subscriptions/deleted_at will now work.

@seancorfield seancorfield added this to the 2.0.0 milestone Aug 13, 2021
@seancorfield seancorfield added the bug It's broken, I'll fix it! label Aug 13, 2021
@devurandom
Copy link
Author

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug It's broken, I'll fix it!
Projects
None yet
Development

No branches or pull requests

2 participants