gh-79579: Update `sqlite3.Cursor.rowcount` for all data-modifying queries by erlend-aasland · Pull Request #93532 · python/cpython

erlend-aasland

🤖 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.

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)

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.

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.

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.

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.

On hold till #93421 is resolved.