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: MSSQL onConflict/merge support #6050

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dustryder
Copy link

Ref: #5646, #3186

What

Support for onConflict/merge for MSSQL

Why

Because there there is no support for this in Knex for MSSQL. You'd have to either decide on an upsert approach and wrap it in raw or use individual statements to achieve the similar behaviour

How

Implementing (most) of the current onConflict/ignore/merge API via a subset of standard ansi sql merge statements. .raw inside onConflict has not been added.

Anything else

Because this approach uses standard syntax, other dialects should also be able implement this in a similar manner (given they too support standard syntax).

test case changes

test changes

cleanup

whoops
@dustryder dustryder marked this pull request as ready for review March 31, 2024 08:13
@000dry
Copy link

000dry commented May 15, 2024

Is anything happening with this? Seems like a no-brainer and would be really helpful to MSSQL users

@KrisLau
Copy link

KrisLau commented Jun 7, 2024

@elhigu @kibertoad Patched my local knex with this and it seems to work

@KrisLau
Copy link

KrisLau commented Jun 13, 2024

@dustryder Been testing it for a little bit and everything works well except for unique composite indexes and tables with triggers. I ended up just using a .del() then .insert in my case since it was just for a seed file but it seems to have been the issue with the original PR as well: #3763 (comment).

Another issue is that I seem to be getting The target table '<table name>' of the DML statement cannot have any enabled triggers if the statement contains an OUTPUT clause without INTO clause. errors which may or may not be related to #4152

@KalebMatthews
Copy link
Contributor

The code in the commit linked related to triggers is only hit if you add the includeTriggerModifications option in your insert or update statement. This just bypasses the output using a slower methodology but it's a simple way to get around the trigger issues for return values. You could use the same ideology for the upset I suspect though. If you need further clarifications let me know.

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.

None yet

4 participants