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

[10.x] Add upsertUsing method to query builder #50305

Closed
wants to merge 9 commits into from

Conversation

nikazooz
Copy link
Contributor

@nikazooz nikazooz commented Feb 28, 2024

Currently we have upsert method for operations where we want to insert new records and update existing ones based on some unique identifier. upsert, similarly to insert, uses provided array of values to insert and update the table.

To insert using a subquery as source of data instead of an array we have insertUsing method. So, in a similar fashion, upsertUsing method that this PR is adding, uses a subquery as source of data to perform "upsert".

I can get something like this for my projects using macros, but I thought that it could be useful to others, hence this PR.

There are no breaking changes, only new functionality.

If there's something to improve before this PR can be accepted, please let me know and I'll make the changes.

Cheers!

@taylorotwell
Copy link
Member

taylorotwell commented Mar 4, 2024

Tried using it on a real SQLite database - failed with invalid syntax:

SQLSTATE[HY000]: General error: 1 near "do": syntax error (Connection: sqlite, SQL: insert into "flights" ("origin", "destination", "duration") select "origin", "destination", "duration" from "old_flights" on conflict ("origin", "destination") do update set "duration" = "excluded"."duration")

DB::table('flights')->upsertUsing(['origin', 'destination', 'duration'], function ($query) {
    $query->select(['origin', 'destination', 'duration'])
        ->from('old_flights');
}, ['origin', 'destination'], ['duration']);

Mark as ready for review if you want me to look again.

@taylorotwell taylorotwell marked this pull request as draft March 4, 2024 18:52
@nikazooz nikazooz marked this pull request as ready for review March 5, 2024 11:41
@nikazooz
Copy link
Contributor Author

nikazooz commented Mar 5, 2024

Found the problem described in the documentation under "2.2. Parsing Ambiguity".

@taylorotwell
Copy link
Member

But it raises questions in my mind of what this has actually been tested against if the most basic example didn't work? 😅

@nikazooz
Copy link
Contributor Author

nikazooz commented Mar 5, 2024

Good point. Somehow I forgot to test against SQLite, but tested against other databases (even MS SQL Server which I haven't used before). I'm now finding out about a limitation with SQL server if f.e. "ORDER BY" is to be used, subquery also needs to contain "OFFSET". I'll convert to draft and mark ready for review once the changes are done.

@nikazooz nikazooz marked this pull request as draft March 5, 2024 15:26
@driesvints
Copy link
Member

Please send this to 11.x instead, thanks.

@driesvints driesvints closed this Mar 12, 2024
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