← 返回首页
gh-79579: Update `sqlite3.Cursor.rowcount` for all data-modifying queries by erlend-aasland · Pull Request #93532 · python/cpython · GitHub
Skip to content

Navigation Menu

Toggle navigation
Sign in
Appearance settings
Search or jump to...

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Include my email address so I can be contacted
Cancel Submit feedback

Saved searches

Use saved searches to filter your results more quickly

Cancel Create saved search
Appearance settings
Resetting focus

gh-79579: Update sqlite3.Cursor.rowcount for all data-modifying queries#93532

Closed
erlend-aasland wants to merge 1 commit into
python:mainfrom
erlend-aasland:sqlite-rowcount/gh-79579
Closed

gh-79579: Update sqlite3.Cursor.rowcount for all data-modifying queries#93532
erlend-aasland wants to merge 1 commit into
python:mainfrom
erlend-aasland:sqlite-rowcount/gh-79579

Conversation

erlend-aasland commented Jun 6, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

Resolves #79579

erlend-aasland commented Jun 6, 2022

Copy link
Copy Markdown
Contributor Author

@animalize, would you like to review this?

erlend-aasland added needs backport to 3.10 only security fixes needs backport to 3.11 only security fixes 🔨 test-with-buildbots Test PR w/ buildbots; report in status section labels Jun 6, 2022

bedevere-bot commented Jun 6, 2022

Copy link
Copy Markdown

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

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

bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jun 6, 2022

erlend-aasland commented Jun 6, 2022

Copy link
Copy Markdown
Contributor Author

cc. @zzzeek

ghost commented Jun 6, 2022 •
edited by ghost
Loading

Copy link
Copy Markdown

Before this change, this code prints-1.
After this change, it prints 1, it seems wrong.
IMO #10913 's method is clear, it skips the leading comments when detecting dml statement.

import sqlite3 cursor = sqlite3.connect(":memory:", isolation_level=None).cursor() cursor.execute("""CREATE TABLE foo (id INTEGER)""") values = ((1,), (2,), (3,)) cursor.executemany("""INSERT INTO foo (id) VALUES (?)""", values) cursor.execute("vacuum") print(cursor.rowcount)

erlend-aasland commented Jun 6, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

After this change, it prints 1, it seems wrong.

It seems wrong is a bit vague. Would you mind to elaborate?

Actually, this will be fixed by #93520 (which currently also encompasses this change), so it does not matter. IMO, trying to parse comments is fragile approach, and I would not recommend such a solution.

ghost commented Jun 6, 2022

Copy link
Copy Markdown

It seems wrong is a bit vague. Would you mind to elaborate?

In .rowcount doc:

As required by the Python DB API Spec, the rowcount attribute “is -1 in case no executeXX() has been performed on the cursor or the rowcount of the last operation is not determinable by the interface”.

IMHO, using sqlite3_stmt_readonly() to detect statement type is unreliable. It return false when the database may be modified, this may include some irrelevant statements, then give a suprise in some cases, for example:
https://bugs.python.org/issue28518
So I suggest not to use sqlite3_stmt_readonly() to detect statement type. I saw you used it in 878e726, but it's only an error message, not involving precise data.

ghost commented Jun 6, 2022

Copy link
Copy Markdown

IMO, trying to parse comments is fragile approach, and I would not recommend such a solution.

Yes, but IMO this is the most reliable way.
If just skip the leading comments and whitespaces, it shouldn't be very complicated.

erlend-aasland commented Jun 6, 2022

Copy link
Copy Markdown
Contributor Author

It return false when the database may be modified, this may include some irrelevant statements, then give a suprise in some cases, for example:
https://bugs.python.org/issue28518

I don't think that is a valid comparison. We're not talking about transaction control here; just setting .rowcount or not. IMO, this is the best way. I don't see any problems with DB API here.

erlend-aasland marked this pull request as draft June 6, 2022 21:57

erlend-aasland commented Jun 6, 2022

Copy link
Copy Markdown
Contributor Author

On hold till #93421 is resolved.

erlend-aasland commented Jun 8, 2022

Copy link
Copy Markdown
Contributor Author

Closing in favour of #93623

erlend-aasland deleted the sqlite-rowcount/gh-79579 branch June 8, 2022 23:50
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

berkerpeksag Awaiting requested review from berkerpeksag
serhiy-storchaka Awaiting requested review from serhiy-storchaka

Assignees

No one assigned

Labels

awaiting core review needs backport to 3.10 only security fixes needs backport to 3.11 only security fixes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

SQLite incorrect row count for UPDATE

2 participants

Footer

© 2026 GitHub, Inc.