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

[8.x] Document the new upsert method #6466

Merged
merged 1 commit into from Oct 7, 2020
Merged

[8.x] Document the new upsert method #6466

merged 1 commit into from Oct 7, 2020

Conversation

paras-malhotra
Copy link
Contributor

Documents the new upsert method on the Eloquent and base query builders based on laravel/framework#34698

@taylorotwell
Copy link
Member

taylorotwell commented Oct 6, 2020

I'm not sure this feature is even working correctly. For inserted rows no created_at timestamp is created. Also, for some reason when I do an upsert which updates a single row and inserts a single row, the upsert function returns 3.

If my model uses both created_at and updated_at timestamps, how am I supposed to reliably set the created_at timestamp?

May need to be reverted.

@paras-malhotra
Copy link
Contributor Author

@taylorotwell I'll modify the PR to support created_at timestamps as well. It isn't supported right now. It's very strange that the insert+update returns 3 records. The SQL generated is backed by tests in the PR and the SQL engine just returns the number of records, so not sure why it would return 3. You can probably re-check after the follow-up PR.

@taylorotwell
Copy link
Member

How will you support created_at though? If you add it to every record it will update even if the record already exists and has a created_at timestamp.

@paras-malhotra
Copy link
Contributor Author

@taylorotwell, we can modify the signature of upsert to upsert($values, $uniqueBy, $update) where the first argument is the values to insert and the 3rd argument is the values to update. So, on duplicate key update, only the update values will be updated and created_at would not be listed as an update value.

@taylorotwell
Copy link
Member

OK

@taylorotwell taylorotwell merged commit d581efd into laravel:8.x Oct 7, 2020
@paras-malhotra paras-malhotra deleted the document_upsert branch October 7, 2020 16:51
@MZanggl
Copy link

MZanggl commented Oct 8, 2020

Sorry to post on this merged PR, but I wonder if it would be good to include a warning here that this can cause huge gaps in primary keys.
Say you have a batch that hourly upserts tens of thousands of records (in which most are updates). At least with MySQL, for every record, it will increment the primary key regardless if the record exists or not (reference: https://stackoverflow.com/questions/38347110/prevent-innodb-auto-increment-on-duplicate-key).

It's an edge case really, as it only becomes an issue in scenarios like described above, because the PK will eventually exceed the limit.

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