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] Use default INSERT INTO syntax for multiple records with SQLite #25995

Merged
merged 1 commit into from
Oct 8, 2018

Conversation

dos1
Copy link
Contributor

@dos1 dos1 commented Oct 7, 2018

This bumps the version of SQLite dependency to 3.7.11, released March 2012.

Using a default syntax for the INSERT INTO clause fixes an issue described in the bug #25262, but only when used with SQLite 3.8.8 (released January 2015) or newer.

Closes #25262

(sending against the master branch because of the version dependency)

@staudenmeir
Copy link
Contributor

Removing compileInsert() breaks empty inserts. We have to use the code from PostgresGrammar::compileInsert().

@dos1
Copy link
Contributor Author

dos1 commented Oct 8, 2018

Right, I have noticed the tests failing. Running them locally with --filter Database and --filter SQLite turned out to not be enough :)

Thanks for pointing to PostgresGrammar. I'll update this PR shortly.

This bumps the version of SQLite dependency to 3.7.11, released
March 2012.

Using a default syntax for the INSERT INTO clause fixes an issue
described in the bug laravel#25262, but only when used with SQLite 3.8.8
(released January 2015) or newer.

The DEFAULT VALUES case needs to be still handled though, just like
in the PostgresGrammar.

Closes laravel#25262
@dos1
Copy link
Contributor Author

dos1 commented Oct 8, 2018

Updated :)

@TBlindaruk TBlindaruk changed the title Use default INSERT INTO syntax for multiple records with SQLite [5.8] Use default INSERT INTO syntax for multiple records with SQLite Oct 8, 2018
@taylorotwell taylorotwell merged commit 95c2939 into laravel:master Oct 8, 2018
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