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

feat: support UPDATE ... FROM ... syntax #833

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

the-wondersmith
Copy link

PR Info

  • Partially addresses #608

New Features

  • Adds support for explicitly specifying a FROM clause in an UPDATE query
    • e.x. UPDATE table1 SET some_column = table2.another_column FROM table2 WHERE table1.another_column = table2.a_different_column

@the-wondersmith
Copy link
Author

Just tagging @tyt2y3 @billy1624 (as the most active/recent mergers) to try and get some traction on this 😅

@tyt2y3
Copy link
Member

tyt2y3 commented Dec 1, 2024

Thank you for the PR! sadly UPDATE ... FROM is not valid syntax on MySQL.
I think it is also supported in SQLite, so it's worth adding, but may be we need to indicate this in the docs

@the-wondersmith
Copy link
Author

the-wondersmith commented Dec 1, 2024

Thank you for the PR! sadly UPDATE ... FROM is not valid syntax on MySQL. I think it is also supported in SQLite, so it's worth adding, but may be we need to indicate this in the docs

Totally valid. We can restrict it by feature flag, if that works? So it's only available when the proper feature flags are enabled, and the emitted queries are syntactically correct for the enabled feature-set.

Out of curiosity, do you happen to know the equivalent syntax for MySQL? Once-upon-a-time I was extremely familiar with the major flavors of SQL syntax, as I was doing a lot of tinkering in the internals of SQLAlchemy due to being the maintainer for a couple of esoteric dialects for that package. In the intervening time, I've transitioned to rust almost completely, so tl;dr SeaORM/SeaQuery are the gotos AFAIC 😅

Anyway, if you're amenable to the update, I'll sort out the syntax / engine pairings and update the PR.

///
/// assert_eq!(
/// query.to_string(MysqlQueryBuilder),
/// "UPDATE `glyph` SET `tokens` = `character`.`character` FROM `character` WHERE `glyph`.`image` = `character`.`user_data`"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The syntax of MySQL appears similar, but not identical. Here a MySQL UPDATE JOIN could be used to achieve the same:

UPDATE `glyph`
JOIN `character`
  ON `glyph`.`image` = `character`.`user_data`
SET `glyph`.`tokens` = `character`.`character`;

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gronke Took a wild swing at at least getting started here, and extracted the specific behavior into a discrete method (prepare_update_from_table_refs) so it can be overridden as needed by whichever dialect. I've got the MySQL-specific handling as a unimplemented! call at the moment though, cause I've got a case of end-of-year-holidays-brain.

If you have a second, would you take a look at the changes and let me know if there's a better / more preferable way of doing this please let me know? Otherwise I'll carry on as time allows 😁

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

Successfully merging this pull request may close these issues.

3 participants