Sitelet https://github.com/python/cpython/issues/79579
Skip to content

SQLite incorrect row count for UPDATE #79579

Closed
#93623
Closed
SQLite incorrect row count for UPDATE#79579
#93623
Labels
3.7 (EOL)end of life3.8 (EOL)end of lifestdlibStandard Library Python modules in the Lib/ directorytopic-sqlite3type-bugAn unexpected behavior, bug, or error

Description

@montanalow
montanalow
mannequin
opened on Dec 4, 2018
BPO 35398
Nosy @berkerpeksag, @lysnikolaou, @tirkarthi, @erlend-aasland
PRs
  • bpo-35398: Improve DDL statement detection for SQLite #10913
    		•

    Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    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'

    bugs.python.org fields:

    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

      @montanalow
      montanalowmannequin
      on Dec 4, 2018
      MannequinAuthor

      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/ directory
      on Dec 4, 2018

    3. tirkarthi commented on Dec 4, 2018

      @tirkarthi
      tirkarthi
      on Dec 4, 2018
      Member

      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 life
      3.8 (EOL)end of life
      on Dec 4, 2018

    5. montanalow commented on Dec 5, 2018

      @montanalow
      montanalowmannequin
      on Dec 5, 2018
      MannequinAuthor

      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
      erlend-aasland
      on Aug 11, 2021
      Contributor

      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 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
      erlend-aasland
      on Jun 6, 2022
      Contributor

      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
      erlend-aasland
      on Jun 8, 2022
      Contributor

      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
      erlend-aasland
      on Jun 14, 2022
      Contributor

      Fixed by:

    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
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Metadata

    Metadata

    Assignees

    No one assigned

      Labels

      3.7 (EOL)end of life3.8 (EOL)end of lifestdlibStandard Library Python modules in the Lib/ directorytopic-sqlite3type-bugAn unexpected behavior, bug, or error

      Projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions