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

[5.8] Add INSERT IGNORE support to Query Builder #29572

Closed
wants to merge 17 commits into from
Closed

[5.8] Add INSERT IGNORE support to Query Builder #29572

wants to merge 17 commits into from

Conversation

dunhamjared
Copy link
Contributor

@dunhamjared dunhamjared commented Aug 14, 2019

This pull request adds insert or ignore support to Query Builder for the following database engines:

  • MySQL - uses: insert ignore
  • Postgres - uses: on conflict do nothing
  • SQLite - uses: insert or ignore
  • SqlServer - uses: merge

Example:

DB::table('test')->insertOrIgnore([['id'=>1], ['id'=>2]]);

Others have voiced support for this feature: #9612
Also fixes laravel/ideas#1762

@staudenmeir
Copy link
Contributor

Your test is incorrect: MySQL uses backticks instead of double quotes (as shown in the error message).

@dunhamjared dunhamjared changed the title Added INSERT IGNORE support to Query Builder Add INSERT IGNORE support to Query Builder Aug 15, 2019
@staudenmeir
Copy link
Contributor

  • compileInsertIgnore() should reuse compileInsert() instead of duplicating the code.
  • I think it makes more sense for insertIgnore() to return the number of inserted rows (example).
  • It would be possible to support SQL Server with the MERGE statement, but the syntax is more complex and would require insertIgnore() to have an additional parameter (example).

@garygreen
Copy link
Contributor

This is pretty cool, though I can't say I've personally reached for insert-ignore all that often. It assumes that on key conflict you are no longer wanting to update/refresh the values your inserting - this is often not what you want.

A more useful strategy is INSERT...ON DUPLICATE KEY UPDATE as suggested in this stackoverflow post: https://stackoverflow.com/a/548570/63523 -- now that is something I've wanted for a long time in Laravel. 😃

@dunhamjared
Copy link
Contributor Author

I use INSERT IGNORE in conjunction with UNIQUE constraints.

For example:

CREATE TABLE `test` (
	`id` INT(11) NOT NULL AUTO_INCREMENT,
	`foo` VARCHAR(50) NOT NULL DEFAULT '',
	`bar` VARCHAR(50) NOT NULL DEFAULT '',
	PRIMARY KEY (`id`),
	UNIQUE INDEX `foo_bar` (`foo`, `bar`)
);
INSERT IGNORE INTO test (foo, bar) VALUES ('A', 'A'), ('A', 'B'), ('A', 'C');
INSERT IGNORE INTO test (foo, bar) VALUES ('B', 'A'), ('A', 'B'), ('B', 'C');

No need to use INSERT...ON DUPLICATE KEY UPDATE -- but I do see the benefits in other situations.

@garygreen
Copy link
Contributor

Yeah it's useful in cases like that. Maybe the method could be called insertOrIgnore then that opens up the possibility of a insertOrUpdate method which uses the ON DUPLICATE KEY UPDATE strategy.

insertIgnore reads a bit strange to me

@driesvints driesvints changed the title Add INSERT IGNORE support to Query Builder [5.8] Add INSERT IGNORE support to Query Builder Aug 15, 2019
@dunhamjared
Copy link
Contributor Author

@garygreen The method should now read insertOrIgnore instead of insertIgnore. :)

@dunhamjared
Copy link
Contributor Author

@staudenmeir Thank you for the input.

  • Reduced duplicated code
  • insertOrIgnore now returns the number of inserted rows
  • Added SQL Server support with help from your linked example

Co-Authored-By: Jonas Staudenmeir <mail@jonas-staudenmeir.de>
@taylorotwell
Copy link
Member

Resubmit without SQL Server support for this. It's the least used driver and I'm not adding parameters to the query builder just to support SQL Server.

@garygreen
Copy link
Contributor

garygreen commented Aug 19, 2019

@taylorotwell probably wasn't nesscary to close the PR, could of just asked the OP to amend. Jared has been very been responsive in suggestions.

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.

Add INSERT IGNORE support to Query Builder
4 participants