Skip to content

SQLite3 support use returning statement#42955

Closed
OuYangJinTing wants to merge 1 commit intorails:mainfrom
OuYangJinTing:sqlite-support-returning-statement
Closed

SQLite3 support use returning statement#42955
OuYangJinTing wants to merge 1 commit intorails:mainfrom
OuYangJinTing:sqlite-support-returning-statement

Conversation

@OuYangJinTing
Copy link
Contributor

@OuYangJinTing OuYangJinTing commented Aug 6, 2021 •
edited
Loading

Summary

SQLite(3.35.0) release information
The SQLite add support for the RETURNING clause on DELETE, INSERT, and UPDATE statements.
This PR used this feature, as follows:

  • ActiveRecord::InsertAll support use to returning of feature if your SQLite version >= 3.35.0.
  • ActiveRecord::ConnectionAdapters::AbstractAdapter#exec_insert support return primary key of value directly. If you want to keep original behavior, you can set insert_returning: false in connection configuration of database.

PS: English is not my native language; please excuse typing errors.

@OuYangJinTing OuYangJinTing force-pushed the sqlite-support-returning-statement branch 2 times, most recently from 06b4fe6 to 652cac8 Compare August 6, 2021 12:14
@OuYangJinTing
Copy link
Contributor Author

OuYangJinTing commented Aug 6, 2021 •
edited
Loading

I have a look CI test and I do not found SQLite's version information.
It may not be able to test fot the SQLite‘s returning caluse feature and maybe need use newer SQLite.

I run test is pass locally and I use 3.36.0 SQLite.

@OuYangJinTing OuYangJinTing force-pushed the sqlite-support-returning-statement branch 2 times, most recently from 012decc to ce848d5 Compare August 6, 2021 19:32
@OuYangJinTing OuYangJinTing changed the title ActiveRecord: sqlite3 support use returning statement SQLite3 support use returning statement Aug 6, 2021
@OuYangJinTing OuYangJinTing force-pushed the sqlite-support-returning-statement branch from ce848d5 to e7dea91 Compare August 6, 2021 19:42
@simi
Copy link
Contributor

simi commented Aug 29, 2021

If I understand it well, CI is running in Docker image based on Debian 11 and installing Sqlite3 using system package.

https://packages.debian.org/bullseye/sqlite3

Currently it is using 3.34.1-3 version.

@jrochkind jrochkind mentioned this pull request Oct 18, 2021
Closed
@OuYangJinTing
Copy link
Contributor Author

OuYangJinTing commented Oct 19, 2021

If I understand it well, CI is running in Docker image based on Debian 11 and installing Sqlite3 using system package.

https://packages.debian.org/bullseye/sqlite3

Currently it is using 3.34.1-3 version.

Thanks for your reply.
Maybe we should wait for the sqlite3 version to upgrade, and then look at this PR.

@rails-bot
Copy link

rails-bot bot commented Jan 17, 2022

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
Thank you for your contributions.

@rails-bot rails-bot bot added the stale label Jan 17, 2022
@rails-bot rails-bot bot closed this Jan 24, 2022
@OuYangJinTing OuYangJinTing mentioned this pull request Feb 23, 2022
Closed
@yahonda
Copy link
Member

yahonda commented May 23, 2022

Reopened another closed by stale label.

@yahonda yahonda reopened this May 23, 2022
@rails-bot rails-bot bot removed the stale label May 23, 2022
@yahonda
Copy link
Member

yahonda commented May 23, 2022

@OuYangJinTing Would you rebase this pull request based on the latest main branch if you are still interested in. Thanks.

@yahonda yahonda mentioned this pull request May 23, 2022
Closed
ghiculescu
ghiculescu reviewed Aug 2, 2022
View reviewed changes
activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb Outdated
Copy link
Member

@ghiculescu ghiculescu Aug 2, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# defaults to true.(Only SQLite3's version >= 3.35.0)
# defaults to true. (Only SQLite3 version >= 3.35.0)

Copy link
Contributor Author

@OuYangJinTing OuYangJinTing Nov 4, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

@OuYangJinTing OuYangJinTing force-pushed the sqlite-support-returning-statement branch from e7dea91 to 9037cc9 Compare November 4, 2022 16:29
@OuYangJinTing OuYangJinTing force-pushed the sqlite-support-returning-statement branch from 9037cc9 to 4749604 Compare November 4, 2022 16:39
@OuYangJinTing
Copy link
Contributor Author

OuYangJinTing commented Nov 4, 2022 •
edited
Loading

@yahonda
I re-checked and updated the commit. Can you help to re-review? thank you!
I will add returning statement ability for mariadb if this PR can be merged

@benedikt benedikt mentioned this pull request Jan 27, 2023
Open
4 tasks
@nvasilevski
Copy link
Contributor

nvasilevski commented Sep 26, 2023

Hey folks, I'd like to see if #49290 achieved everything we aimed for in this PR so this one could be closed.

@fractaledmind fractaledmind mentioned this pull request Dec 28, 2023
Merged
4 tasks
@fractaledmind
Copy link
Contributor

fractaledmind commented Dec 28, 2023

@nvasilevski: I opened #50468 to add the documentation improvements, but everything else in this PR was captured by #49290. Once #50468 is merged, this can be closed. @OuYangJinTing, I made sure to make you a co-author of the #50468 PR. Thanks for your good work here.

@skipkayhil
Copy link
Member

skipkayhil commented Dec 28, 2023

Closing as completed by #49290 and #50468

@skipkayhil skipkayhil closed this Dec 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ghiculescu ghiculescu Awaiting requested review from ghiculescu

Assignees

No one assigned

Labels

activerecord

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@OuYangJinTing @simi @yahonda @nvasilevski @fractaledmind @skipkayhil @ghiculescu