Description

assignee = None
closed_at = None
created_at = <Date 2018-12-04.02:27:45.503>
labels = ['3.8', '3.7', 'library']
title = 'SQLite incorrect row count for UPDATE'
updated_at = <Date 2021-08-11.21:01:29.425>
user = 'https://bugs.python.org/MontanaLow'

activity = <Date 2021-08-11.21:01:29.425>
actor = 'erlendaasland'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)']
creation = <Date 2018-12-04.02:27:45.503>
creator = 'Montana Low'
dependencies = []
files = []
hgrepos = []
issue_num = 35398
keywords = ['patch']
message_count = 4.0
messages = ['331006', '331069', '331090', '399417']
nosy_count = 7.0
nosy_names = ['ghaering', 'zzzeek', 'berker.peksag', 'lys.nikolaou', 'xtreak', 'Montana Low', 'erlendaasland']
pr_nums = ['10913']
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = None
url = 'https://bugs.python.org/issue35398'
versions = ['Python 2.7', 'Python 3.6', 'Python 3.7', 'Python 3.8']

Activity

1. montanalow commented on Dec 4, 2018

   

   montanalowmannequin

   on Dec 4, 2018

   MannequinAuthor

   More actions

   SQLite driver returns an incorrect row count (-1) for UPDATE statements that begin with a comment.

   Downstream Reference:
   sqlalchemy/sqlalchemy#4396

   Test Case:

   import sqlite3
   
   conn = sqlite3.connect(":memory:")
   
   cursor = conn.cursor()
   
   cursor.execute("""
   CREATE TABLE foo (
       id INTEGER NOT NULL,
       updated_at DATETIME,
       PRIMARY KEY (id)
   )
   """)
   
   cursor.execute("""
   /* watermarking bug */
   INSERT INTO foo (id, updated_at) VALUES (?, ?)
   """, [1, None])
   
   
   cursor.execute("""
       UPDATE foo SET updated_at=? WHERE foo.id = ?
   """, ('2018-12-02 14:55:57.169785', 1))
   
   assert cursor.rowcount == 1
   
   cursor.execute("""
       /* watermarking bug */
       UPDATE foo SET updated_at=? WHERE foo.id = ?
   """, ('2018-12-03 14:55:57.169785', 1))
   
   assert cursor.rowcount == 1
   
2. montanalow

   added

   stdlibStandard Library Python modules in the Lib/ directoryStandard Library Python modules in the Lib/ directory

   on Dec 4, 2018
3. tirkarthi commented on Dec 4, 2018

   

   tirkarthi

   on Dec 4, 2018

   Member

   More actions

   Did some debugging here. If I am understanding this correctly the rowcount is set at

   cpython/Modules/_sqlite/cursor.c

   Line 574 in b8e689a

   if (self->statement->is_dml) {

   . It checks for is_dml flag that is set here

   cpython/Modules/_sqlite/statement.c

   Line 78 in b8e689a

   self->is_dml = 0;

   The part to set is_dml skips space, tabs and newline and checks for the first set of characters that is not skipped to be insert, update, delete or replace and in this case the first set of characters to be matched will be "/* watermarking */". Thus with comment not matching, is_dml is not set and -1 is set for the rowcount.
4. tirkarthi

   added

   3.7 (EOL)end of lifeend of life

   3.8 (EOL)end of lifeend of life

   on Dec 4, 2018
5. montanalow commented on Dec 5, 2018

   

   montanalowmannequin

   on Dec 5, 2018

   MannequinAuthor

   More actions

   I took a stab at patch. It fixes the issue for me, as proven via the Test Case.
6. erlend-aasland commented on Aug 11, 2021

   

   erlend-aasland

   on Aug 11, 2021

   Contributor

   More actions

   See also gh-81040.
7. ezio-melotti

   transferred this issue fromon Apr 10, 2022
8. erlend-aasland

   added

   topic-sqlite3

   type-bugAn unexpected behavior, bug, or errorAn unexpected behavior, bug, or error

   on May 16, 2022
9. erlend-aasland

   added this to sqlite3 issueson May 21, 2022
10. erlend-aasland

    moved this to In Progress in sqlite3 issueson Jun 5, 2022
11. erlend-aasland commented on Jun 6, 2022

    

    erlend-aasland

    on Jun 6, 2022

    Contributor

    More actions

    Now that we require SQLite 3.7.15, we can safely use sqlite3_stmt_readonly to determine if we should update .rowcount or not. The API returns true for SELECT queries, which is also the case today. The API is pretty conservative, and returns false for cases where a query might modify the database. In those cases, we may end up with .rowcount being zero. That is perfectly fine.

    I see no reason not to use sqlite3_stmt_readonly to solve this.

    EDIT:

    sqlite3_changes support INSERT, UPDATE, and DELETE statements only. sqlite3_stmt_readonly encompasses a lot more than that. As long as we are using sqlite3_changes to update .rowcount, we simply cannot use sqlite3_stmt_readonly, because SQLite does not reset the number returned by sqlite3_changes; for unsupported statements, you may end up with sqlite3_changes returning the change count for a previous statement.

    If we'd used something like sqlite3_total_changes64, we could safely use sqlite3_stmt_readonly, because in the worst cases we'd end up with a .rowcount of zero, and that is perfectly fine.
12. erlend-aasland

    added a commit that references this issue on Jun 6, 2022

    pythongh-79579: Update sqlite3.Cursor.rowcount for all datamodifying …

    e82adb8
13. erlend-aasland commented on Jun 8, 2022

    

    erlend-aasland

    on Jun 8, 2022

    Contributor

    More actions

    I see no reason not to use sqlite3_stmt_readonly to solve this.

    @animalize made me change my mind about that, so I made a competing PR: gh-93623

    Thanks for pushing, Ma Lin.
14. erlend-aasland

    added a commit that references this issue on Jun 13, 2022

    Merge branch 'main' into sqlite-rowcount/pythongh-79579-alt

    adcba2c
15. erlend-aasland

    closed this as completedin #93623on Jun 14, 2022
16. Repository owner moved this from In Progress to Done in sqlite3 issueson Jun 14, 2022
17. erlend-aasland

    added 3 commits that reference this issue on Jun 14, 2022

    gh-79579: Improve DML query detection in sqlite3 (#93623)

    4674007

    [3.11] pythongh-79579: Improve DML query detection in sqlite3 (python…

    a521b3c

    [3.10] pythongh-79579: Improve DML query detection in sqlite3 (python…

    84423c9
18. erlend-aasland commented on Jun 14, 2022

    

    erlend-aasland

    on Jun 14, 2022

    Contributor

    More actions

    Fixed by:

    • main: gh-79579: Improve DML query detection in sqlite3 #93623
    • 3.11: [3.11] gh-79579: Improve DML query detection in sqlite3 (GH-93623) #93800
    • 3.10: [3.10] gh-79579: Improve DML query detection in sqlite3 (GH-93623) #93801
19. erlend-aasland

    added 2 commits that reference this issue on Jun 14, 2022

    [3.11] gh-79579: Improve DML query detection in sqlite3 (GH-93623) (#…

    871b1dc

    [3.10] gh-79579: Improve DML query detection in sqlite3 (GH-93623) (#…

    2229d34
20. serhiy-storchaka

    added a commit that references this issue on Jun 26, 2022

    Revert 30935 defer bpo45162 to 312 (#94285)

    1ffde3e
21. cfbolz

    added a commit that references this issue on Dec 2, 2024

    sqlite3: deal with comments at the start of the query

    5b219d4

Issue actions