Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sqlite3 SELECT does not BEGIN a transaction, but should according to spec #54133

Closed
zzzeek mannequin opened this issue Sep 23, 2010 · 8 comments
Closed

sqlite3 SELECT does not BEGIN a transaction, but should according to spec #54133

zzzeek mannequin opened this issue Sep 23, 2010 · 8 comments
Labels
stdlib Python modules in the Lib dir topic-sqlite3 type-bug An unexpected behavior, bug, or error

Comments

@zzzeek
Copy link
Mannequin

zzzeek mannequin commented Sep 23, 2010

BPO 9924
Nosy @pitrou, @bitdancer, @dholth, @berkerpeksag, @palaviv, @erlend-aasland
Files
  • unintuitive_sqlite_behavior.py
  • 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 2010-09-23.03:56:05.034>
    labels = ['3.7', 'type-bug', 'library']
    title = 'sqlite3 SELECT does not BEGIN a transaction, but should according to spec'
    updated_at = <Date 2021-05-19.11:16:15.521>
    user = 'https://bugs.python.org/zzzeek'

    bugs.python.org fields:

    activity = <Date 2021-05-19.11:16:15.521>
    actor = 'erlendaasland'
    assignee = 'none'
    closed = False
    closed_date = None
    closer = None
    components = ['Library (Lib)']
    creation = <Date 2010-09-23.03:56:05.034>
    creator = 'zzzeek'
    dependencies = []
    files = ['42260']
    hgrepos = []
    issue_num = 9924
    keywords = []
    message_count = 7.0
    messages = ['117167', '117168', '117180', '129128', '207794', '262284', '307684']
    nosy_count = 11.0
    nosy_names = ['ghaering', 'pitrou', 'rnortman', 'r.david.murray', 'zzzeek', 'dholth', 'berker.peksag', 'palaviv', 'rhunter', 'Allen Li', 'erlendaasland']
    pr_nums = []
    priority = 'normal'
    resolution = None
    stage = None
    status = 'open'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue9924'
    versions = ['Python 3.6', 'Python 3.7']

    @zzzeek
    Copy link
    Mannequin Author

    zzzeek mannequin commented Sep 23, 2010

    Copying this bug from the pysqlite tracker, at http://code.google.com/p/pysqlite/issues/detail?id=21 , as the issue has been opened for two days with no reply. (side node - should sqlite3 bugs be reported here or on the pysqlite tracker ?) The text below was originally written by Randall Nortman:

    Pysqlite does not open a transaction in the database until a DML statement is encountered (INSERT, UPDATE, or DELETE). A DQL (SELECT) statement will not cause a transaction to be opened if one is not already opened. This is the documented behavior, but it is not what is intended by the spec (PEP-249). The spec intends a transaction to always be open (per the spec author), and this is what happens in other DB-API drivers. For more information, see the this DB-SIG mailing list post (by the PEP-249 author):

    http://mail.python.org/pipermail/db-sig/2010-September/005645.html

    For additional background, see this thread on the SQLAlchemy mailing list, which is the source of the attached test case:

    http://groups.google.com/group/sqlalchemy/browse_thread/thread/2f47e28c1fcdf9e6/0ef1666759ce0724#0ef1666759ce0724

    What steps will reproduce the problem?

    1. See attached test case. Run it as is, and the final conn1.commit() statement will complete successfully.
    2. Uncomment the c2.execute("BEGIN") line and run again; this time conn1.commit() hangs until a timeout, then a "Database is locked" error is returned.

    What is the expected output? What do you see instead?

    The BEGIN should be issued implicitly, and even without doing it explicitly, the commit should block and then return the DB locked error.

    What version of the product are you using? On what operating system?

    Python 2.6.6 with its built-in sqlite3 module, on Debian Squeeze x86.

    import sqlite3
    import os
    
    if os.path.exists("file.db"):
        os.unlink("file.db")
        
    conn1 = sqlite3.connect("file.db")
    
    c1 = conn1.cursor()
    
    c1.execute("PRAGMA read_uncommitted=SERIALIZABLE")

    c1.execute("""create table foo (id integer primary key, data varchar(30))""")
    c1.execute("insert into foo(id, data) values (1, 'data1')")
    c1.close()
    conn1.commit()

    c1 = conn1.cursor()
    c1.execute("select * from foo where id=1")
    row1 = c1.fetchone()
    c1.close()
    
    conn2 = sqlite3.connect("file.db")
    c2 = conn2.cursor()
    
    c2.execute("PRAGMA read_uncommitted=SERIALIZABLE")
    
    # sqlite3 should be doing this automatically.
    # when called, conn1's commit blocks
    #c2.execute("BEGIN")
    c2.execute("select * from foo where id=1")
    row2 = c2.fetchone()
    c2.close()
    
    c1 = conn1.cursor()
    c1.execute("update foo set data='data2'")

    print "About to commit conn1..."
    conn1.commit()

    @zzzeek zzzeek mannequin added stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels Sep 23, 2010
    @zzzeek
    Copy link
    Mannequin Author

    zzzeek mannequin commented Sep 23, 2010

    My own comment here is that I'm supposing the "late BEGIN" behavior is to cut down on SQLite's file locking. I think a way to maintain that convenience for most cases, while allowing the stricter behavior that makes SERIALIZABLE isolation worthwhile, would be an option to sqlite3.connect() that moves the implicit BEGIN to before any DQL, not just DML, statement.

    @ghaering
    Copy link
    Mannequin

    ghaering mannequin commented Sep 23, 2010

    Yes Mike. Avoiding unnecessary locks was exactly the reason for this behaviour. I agree that for serializable transactions I'd need to make some changes.

    @dholth
    Copy link
    Mannequin

    dholth mannequin commented Feb 22, 2011

    What should this option be called?

    connect(strict=True) ?

    @zzzeek
    Copy link
    Mannequin Author

    zzzeek mannequin commented Jan 9, 2014

    see also http://bugs.python.org/issue10740, which also relates to pysqlite attempting to make guesses as to when transactions should begin and end.

    @ghaering ghaering mannequin self-assigned this Jan 11, 2015
    @rhunter
    Copy link
    Mannequin

    rhunter mannequin commented Mar 23, 2016

    This bug can also lead to subtle and unintuitive "database is locked" bugs, even when a large timeout is set on the connection. Many, many people are affected by this bug (search the web for "python sqlite database is locked"). I've attached code that demonstrates this issue.

    I disagree that the current behavior cuts down on SQLite file locking. As soon as any SELECT statement is opened, an implicit lock is held by SQLite (whether it resides within a BEGIN block or not): https://www.sqlite.org/lockingv3.html

    SQLite has been designed to do its own "late locking." Pysqlite does not need to duplicate this behavior.

    This is a clear-as-day bug and should be fixed.

    @pitrou
    Copy link
    Member

    pitrou commented Dec 5, 2017

    See https://bugs.python.org/issue32215 for what seems to be a related bug.

    Also note that pysqlite now seems to be using a different logic:
    https://github.com/ghaering/pysqlite/blob/master/src/cursor.c#L537-L548
    Also this changeset:
    ghaering/pysqlite@94eae50

    As a sidenote, this seems to mean that the stdlib sqlite module doesn't receive updates anymore from its author...?

    @erlend-aasland
    Copy link
    Contributor

    As of the recent discussion on Discourse, I'm closing this as superseded by the proposed solution in gh-83638.

    TL;DR: We cannot change the behaviour because of backwards compatibility. The proposed fix is to introducing a new, PEP 249 compliant way of controlling transactions. The old transaction handling (isolation_level) will be deprecated.

    @erlend-aasland erlend-aasland closed this as not planned Won't fix, can't repro, duplicate, stale Jun 21, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    stdlib Python modules in the Lib dir topic-sqlite3 type-bug An unexpected behavior, bug, or error
    Projects
    Status: Discarded
    Development

    No branches or pull requests

    2 participants