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

[SQLite] Fix upsert targeting composite keys #521

Merged
merged 2 commits into from
May 28, 2023

Conversation

wkunert
Copy link
Contributor

@wkunert wkunert commented Apr 26, 2023

Fix for #518.

If the target consisted of multiple columns, they were wrapped in two pairs for parentheses, which was invalid SQL. Thanks @hirvesh for identifying the root cause of the failed SQL queries.

Also a target is required with "DO UPDATE" upserts now as per the spec (cp. https://www.sqlite.org/lang_UPSERT.html). Should not break existing code bases since queries without a target would have failed.

If the target consisted of multiple columns, they were wrapped
in two pairs for parentheses, which was invalid SQL.

Also a target is required with "DO UPDATE" upserts now as per the spec
(cp. https://www.sqlite.org/lang_UPSERT.html).
@hirvesh
Copy link

hirvesh commented May 23, 2023

Anything needed to get this merged? @AndriiSherman? 🤔

@AndriiSherman
Copy link
Member

@hirvesh no, we just didn't have time to clean all PRs while releasing 0.26.0

One of the next priorities for @dankochetov would be to review and clean up all PRs, merging everything that appears to be in good condition

So no worries, it all soon should be fixed

@hirvesh
Copy link

hirvesh commented May 24, 2023

Thank you Andrii 🙏

Copy link
Contributor

@dankochetov dankochetov left a comment

Choose a reason for hiding this comment

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

Good stuff 👍

@dankochetov dankochetov merged commit dab4904 into drizzle-team:main May 28, 2023
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.

4 participants