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.

Sign up for GitHub

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

Jump to bottom

bpo-45041: Simplify sqlite3.Cursor.executescript() #28020

Merged
merged 4 commits into from Sep 19, 2021
Merged

bpo-45041: Simplify sqlite3.Cursor.executescript() #28020

merged 4 commits into from Sep 19, 2021

Conversation

erlend-aasland
Copy link
Contributor

@erlend-aasland erlend-aasland commented Aug 28, 2021 •
edited by bedevere-bot

  • increase test coverage for executescript
  • minimise number of times we save/restore thread state
  • optimise SQLite C API usage
  • simplify error handling

https://bugs.python.org/issue45041

Erlend E. Aasland added 2 commits August 28, 2021 23:44
bpo-45041: Simplify sqlite3.Cursor.executemany
a93dcaa 
- minimise number of times we save/restore thread state
- optimise SQLite C API usage
- simplify error handling
Add test for executescript() tx ctrl
f04bd09
@erlend-aasland
Copy link
Contributor Author

This PR greatly simplifies the SQLite statement error path. The sqlite3_stmt * pointer is now kept entirely inside the Py_BEGIN_ALLOW_THREADS/Py_END_ALLOW_THREADS wrapper, so it is very easy to track it from preparation to finalisation.

@erlend-aasland
Copy link
Contributor Author

Code coverage after this PR and #28019:

Filename                      Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
prepare_protocol.c                 20                17    15.00%           4                 3    25.00%          22                15    31.82%
util.c                             65                13    80.00%           3                 0   100.00%          79                15    81.01%
module.c                          321                52    83.80%          11                 1    90.91%         241                52    78.42%
row.c                             216                44    79.63%          13                 1    92.31%         158                19    87.97%
microprotocols.c                   77                 8    89.61%           3                 0   100.00%          98                15    84.69%
connection.c                     1114               119    89.32%          48                 0   100.00%        1278               146    88.58%
module.h                            3                 0   100.00%           3                 0   100.00%          10                 0   100.00%
cursor.c                          796               103    87.06%          23                 0   100.00%         789               105    86.69%
statement.c                       345                20    94.20%          10                 0   100.00%         403                31    92.31%
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                            2957               376    87.28%         118                 5    95.76%        3078               398    87.07%

@erlend-aasland erlend-aasland changed the title bpo-45041: Optimise sqlite3.Cursor.executescript() bpo-45041: Simplify sqlite3.Cursor.executescript() Aug 28, 2021
Revert f04bd09
b92ca72 
This test is added by pythonGH-28074
@erlend-aasland erlend-aasland mentioned this pull request Aug 30, 2021
Merged
@erlend-aasland erlend-aasland added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Sep 1, 2021
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @erlend-aasland for commit 9db16c3 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Sep 1, 2021
@erlend-aasland
Copy link
Contributor Author

erlend-aasland commented Sep 9, 2021 •
edited

The AMD64 Windows8.1 Non-Debug PR and AMD64 Arch Linux Asan Debug PR buildbot failures are unrelated to this PR.

Manually rerunning failed buildbots. See python/buildmaster-config#275

@pablogsal pablogsal merged commit a677971 into python:main Sep 19, 2021
@erlend-aasland erlend-aasland deleted the sqlite-optimise-executemany branch September 19, 2021 22:56
niyas-sait pushed a commit to niyas-sait/cpython that referenced this pull request Sep 21, 2021
@niyas-sait
bpo-45041: Simplify sqlite3.Cursor.executescript() (pythonGH-28020)
2f05d38
@erlend-aasland erlend-aasland mentioned this pull request Sep 21, 2021
Merged
@erlend-aasland erlend-aasland mentioned this pull request May 16, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pablogsal pablogsal pablogsal approved these changes

@berkerpeksag berkerpeksag Awaiting requested review from berkerpeksag berkerpeksag is a code owner

Assignees
No one assigned
Labels
skip news
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@erlend-aasland @bedevere-bot @pablogsal @the-knights-who-say-ni