From bb4062ae874a758305c094ed719f1366ec6a2aa9 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Sun, 5 Jun 2022 09:10:10 +0200 Subject: [PATCH 1/7] Update rowcount after SQLITE_DONE --- Lib/test/test_sqlite3/test_dbapi.py | 8 ++++++++ .../2022-06-05-22-22-42.gh-issue-93421.43UO_8.rst | 2 ++ Modules/_sqlite/cursor.c | 14 +++++++++----- 3 files changed, 19 insertions(+), 5 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2022-06-05-22-22-42.gh-issue-93421.43UO_8.rst diff --git a/Lib/test/test_sqlite3/test_dbapi.py b/Lib/test/test_sqlite3/test_dbapi.py index 1fa02db3b3af41..fb14c0bc71d99e 100644 --- a/Lib/test/test_sqlite3/test_dbapi.py +++ b/Lib/test/test_sqlite3/test_dbapi.py @@ -887,6 +887,14 @@ def test_rowcount_executemany(self): self.cu.executemany("insert into test(name) values (?)", [(1,), (2,), (3,)]) self.assertEqual(self.cu.rowcount, 3) + @unittest.skipIf(sqlite.sqlite_version_info < (3, 35, 0), + "Requires SQLite 3.35.0 or newer") + def test_rowcount_update_returning(self): + # gh-93421: rowcount is updated correctly for UPDATE...RETURNING queries + self.cu.execute("update test set name='bar' where name='foo' returning 1") + self.assertEqual(self.cu.fetchone()[0], 1) + self.assertEqual(self.cu.rowcount, 1) + def test_total_changes(self): self.cu.execute("insert into test(name) values ('foo')") self.cu.execute("insert into test(name) values ('foo')") diff --git a/Misc/NEWS.d/next/Library/2022-06-05-22-22-42.gh-issue-93421.43UO_8.rst b/Misc/NEWS.d/next/Library/2022-06-05-22-22-42.gh-issue-93421.43UO_8.rst new file mode 100644 index 00000000000000..085ea94c03c64e --- /dev/null +++ b/Misc/NEWS.d/next/Library/2022-06-05-22-22-42.gh-issue-93421.43UO_8.rst @@ -0,0 +1,2 @@ +Fix :data:`sqlite3.Cursor.rowcount` for ``UPDATE ... RETURNING``` SQL +queries. Patch by Erlend E. Aasland. diff --git a/Modules/_sqlite/cursor.c b/Modules/_sqlite/cursor.c index c58def5f0362f1..d5c00c41a2abaa 100644 --- a/Modules/_sqlite/cursor.c +++ b/Modules/_sqlite/cursor.c @@ -835,10 +835,9 @@ _pysqlite_query_execute(pysqlite_Cursor* self, int multiple, PyObject* operation stmt_reset(self->statement); } - /* reset description and rowcount */ + /* reset description */ Py_INCREF(Py_None); Py_SETREF(self->description, Py_None); - self->rowcount = 0L; if (self->statement) { (void)stmt_reset(self->statement); @@ -867,6 +866,7 @@ _pysqlite_query_execute(pysqlite_Cursor* self, int multiple, PyObject* operation stmt_reset(self->statement); stmt_mark_dirty(self->statement); + self->rowcount = self->statement->is_dml ? 0L : -1L; /* We start a transaction implicitly before a DML statement. SELECT is the only exception. See #9924. */ @@ -944,13 +944,14 @@ _pysqlite_query_execute(pysqlite_Cursor* self, int multiple, PyObject* operation } } - if (self->statement->is_dml) { + if (self->statement->is_dml && multiple) { self->rowcount += (long)sqlite3_changes(self->connection->db); - } else { - self->rowcount= -1L; } if (rc == SQLITE_DONE && !multiple) { + if (self->statement->is_dml) { + self->rowcount = (long)sqlite3_changes(self->connection->db); + } stmt_reset(self->statement); Py_CLEAR(self->statement); } @@ -1125,6 +1126,9 @@ pysqlite_cursor_iternext(pysqlite_Cursor *self) } int rc = stmt_step(stmt); if (rc == SQLITE_DONE) { + if (self->statement->is_dml) { + self->rowcount = (long)sqlite3_changes(self->connection->db); + } (void)stmt_reset(self->statement); } else if (rc != SQLITE_ROW) { From ce12c963ae00388d6a34bb3236b7590d6018566f Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Mon, 6 Jun 2022 12:28:21 +0200 Subject: [PATCH 2/7] Harden executemany check --- Modules/_sqlite/cursor.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/_sqlite/cursor.c b/Modules/_sqlite/cursor.c index d5c00c41a2abaa..8c59351a0107ee 100644 --- a/Modules/_sqlite/cursor.c +++ b/Modules/_sqlite/cursor.c @@ -944,7 +944,7 @@ _pysqlite_query_execute(pysqlite_Cursor* self, int multiple, PyObject* operation } } - if (self->statement->is_dml && multiple) { + if (self->statement->is_dml && rc == SQLITE_DONE && multiple) { self->rowcount += (long)sqlite3_changes(self->connection->db); } From b66adaf0c378146e819cece19f85a3f18e1380a6 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Mon, 6 Jun 2022 23:05:27 +0200 Subject: [PATCH 3/7] Use 64-bit API if available --- Modules/_sqlite/cursor.c | 16 +++++++++++++--- Modules/_sqlite/cursor.h | 2 +- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/Modules/_sqlite/cursor.c b/Modules/_sqlite/cursor.c index 8c59351a0107ee..77147d25b1fe13 100644 --- a/Modules/_sqlite/cursor.c +++ b/Modules/_sqlite/cursor.c @@ -776,6 +776,16 @@ stmt_mark_dirty(pysqlite_Statement *self) self->in_use = 1; } +static inline sqlite3_int64 +changes(sqlite3 *db) +{ +#if SQLITE_VERSION_NUMBER >= 3037000 + return sqlite3_changes64(db); + #else + return (sqlite3_int64)sqlite3_changes(db); + #endif +} + PyObject * _pysqlite_query_execute(pysqlite_Cursor* self, int multiple, PyObject* operation, PyObject* second_argument) { @@ -945,12 +955,12 @@ _pysqlite_query_execute(pysqlite_Cursor* self, int multiple, PyObject* operation } if (self->statement->is_dml && rc == SQLITE_DONE && multiple) { - self->rowcount += (long)sqlite3_changes(self->connection->db); + self->rowcount += changes(self->connection->db); } if (rc == SQLITE_DONE && !multiple) { if (self->statement->is_dml) { - self->rowcount = (long)sqlite3_changes(self->connection->db); + self->rowcount = changes(self->connection->db); } stmt_reset(self->statement); Py_CLEAR(self->statement); @@ -1127,7 +1137,7 @@ pysqlite_cursor_iternext(pysqlite_Cursor *self) int rc = stmt_step(stmt); if (rc == SQLITE_DONE) { if (self->statement->is_dml) { - self->rowcount = (long)sqlite3_changes(self->connection->db); + self->rowcount = changes(self->connection->db); } (void)stmt_reset(self->statement); } diff --git a/Modules/_sqlite/cursor.h b/Modules/_sqlite/cursor.h index 0bcdddc3e29595..5a9245ddfbea41 100644 --- a/Modules/_sqlite/cursor.h +++ b/Modules/_sqlite/cursor.h @@ -38,7 +38,7 @@ typedef struct PyObject* row_cast_map; int arraysize; PyObject* lastrowid; - long rowcount; + sqlite3_int64 rowcount; PyObject* row_factory; pysqlite_Statement* statement; int closed; From 731dfc59507b88e83e99dea41b3a898772c09f31 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Mon, 6 Jun 2022 23:34:09 +0200 Subject: [PATCH 4/7] Adjust NEWS --- .../Library/2022-06-05-22-22-42.gh-issue-93421.43UO_8.rst | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Misc/NEWS.d/next/Library/2022-06-05-22-22-42.gh-issue-93421.43UO_8.rst b/Misc/NEWS.d/next/Library/2022-06-05-22-22-42.gh-issue-93421.43UO_8.rst index 085ea94c03c64e..9e1d6554e0ab2b 100644 --- a/Misc/NEWS.d/next/Library/2022-06-05-22-22-42.gh-issue-93421.43UO_8.rst +++ b/Misc/NEWS.d/next/Library/2022-06-05-22-22-42.gh-issue-93421.43UO_8.rst @@ -1,2 +1,3 @@ -Fix :data:`sqlite3.Cursor.rowcount` for ``UPDATE ... RETURNING``` SQL -queries. Patch by Erlend E. Aasland. +Update :data:`sqlite3.Cursor.rowcount` when a DML statement has run to +completion. This fixes the row count for SQL queries like +``UPDATE ... RETURNING``. Patch by Erlend E. Aasland. From 02bd83f17f242c091c6f2f1555abdf22d556667c Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Wed, 8 Jun 2022 10:23:37 +0200 Subject: [PATCH 5/7] Revert 64-bit API support --- Modules/_sqlite/cursor.c | 16 +++------------- Modules/_sqlite/cursor.h | 2 +- 2 files changed, 4 insertions(+), 14 deletions(-) diff --git a/Modules/_sqlite/cursor.c b/Modules/_sqlite/cursor.c index 77147d25b1fe13..8c59351a0107ee 100644 --- a/Modules/_sqlite/cursor.c +++ b/Modules/_sqlite/cursor.c @@ -776,16 +776,6 @@ stmt_mark_dirty(pysqlite_Statement *self) self->in_use = 1; } -static inline sqlite3_int64 -changes(sqlite3 *db) -{ -#if SQLITE_VERSION_NUMBER >= 3037000 - return sqlite3_changes64(db); - #else - return (sqlite3_int64)sqlite3_changes(db); - #endif -} - PyObject * _pysqlite_query_execute(pysqlite_Cursor* self, int multiple, PyObject* operation, PyObject* second_argument) { @@ -955,12 +945,12 @@ _pysqlite_query_execute(pysqlite_Cursor* self, int multiple, PyObject* operation } if (self->statement->is_dml && rc == SQLITE_DONE && multiple) { - self->rowcount += changes(self->connection->db); + self->rowcount += (long)sqlite3_changes(self->connection->db); } if (rc == SQLITE_DONE && !multiple) { if (self->statement->is_dml) { - self->rowcount = changes(self->connection->db); + self->rowcount = (long)sqlite3_changes(self->connection->db); } stmt_reset(self->statement); Py_CLEAR(self->statement); @@ -1137,7 +1127,7 @@ pysqlite_cursor_iternext(pysqlite_Cursor *self) int rc = stmt_step(stmt); if (rc == SQLITE_DONE) { if (self->statement->is_dml) { - self->rowcount = changes(self->connection->db); + self->rowcount = (long)sqlite3_changes(self->connection->db); } (void)stmt_reset(self->statement); } diff --git a/Modules/_sqlite/cursor.h b/Modules/_sqlite/cursor.h index 5a9245ddfbea41..0bcdddc3e29595 100644 --- a/Modules/_sqlite/cursor.h +++ b/Modules/_sqlite/cursor.h @@ -38,7 +38,7 @@ typedef struct PyObject* row_cast_map; int arraysize; PyObject* lastrowid; - sqlite3_int64 rowcount; + long rowcount; PyObject* row_factory; pysqlite_Statement* statement; int closed; From b75b725f956b449cfec0544e9c44e3e82d5003b4 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Wed, 8 Jun 2022 10:28:17 +0200 Subject: [PATCH 6/7] Refactor --- Modules/_sqlite/cursor.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/Modules/_sqlite/cursor.c b/Modules/_sqlite/cursor.c index 8c59351a0107ee..d37f164fe15211 100644 --- a/Modules/_sqlite/cursor.c +++ b/Modules/_sqlite/cursor.c @@ -944,10 +944,6 @@ _pysqlite_query_execute(pysqlite_Cursor* self, int multiple, PyObject* operation } } - if (self->statement->is_dml && rc == SQLITE_DONE && multiple) { - self->rowcount += (long)sqlite3_changes(self->connection->db); - } - if (rc == SQLITE_DONE && !multiple) { if (self->statement->is_dml) { self->rowcount = (long)sqlite3_changes(self->connection->db); @@ -958,6 +954,9 @@ _pysqlite_query_execute(pysqlite_Cursor* self, int multiple, PyObject* operation if (multiple) { stmt_reset(self->statement); + if (self->statement->is_dml && rc == SQLITE_DONE) { + self->rowcount += (long)sqlite3_changes(self->connection->db); + } } Py_XDECREF(parameters); } From 0ebf8a8064a3f72e4c2fc3d8b2063d4db22f9022 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Wed, 8 Jun 2022 10:30:45 +0200 Subject: [PATCH 7/7] Doh, reset _after_ fetching changes --- Modules/_sqlite/cursor.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/_sqlite/cursor.c b/Modules/_sqlite/cursor.c index d37f164fe15211..414584d8ee30e9 100644 --- a/Modules/_sqlite/cursor.c +++ b/Modules/_sqlite/cursor.c @@ -953,10 +953,10 @@ _pysqlite_query_execute(pysqlite_Cursor* self, int multiple, PyObject* operation } if (multiple) { - stmt_reset(self->statement); if (self->statement->is_dml && rc == SQLITE_DONE) { self->rowcount += (long)sqlite3_changes(self->connection->db); } + stmt_reset(self->statement); } Py_XDECREF(parameters); }