[issue44092] [sqlite3] consider removing special rollback handling

[Erlend E. Aasland]

Erlend E. Aasland  added the comment:

I've crafted a number of rollback tests, but it occurred to me that they are 
simply just testing SQLite behaviour; not sqlite3 behaviour. I had to adjust 
the tests according to which version of SQLite was used (for example 3.8.7.2 
introduced new behaviour). Such tests are bound to break as SQLite evolves. I'm 
not sure we want such tests in our test suite; it can make the CI fail for 
completely unrelated PRs. Suggesting to leave detailed rollback testing to 
SQLite and just keep a couple of basic tests in our suite.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue44092] [sqlite3] consider removing special rollback handling

[Erlend E. Aasland]

Erlend E. Aasland  added the comment:

The effect of PR 26026 is that InterfaceError is no longer raised for fetch 
across rollback; instead it is up to SQLite how to handle this:

- for some cases, SQLITE_ABORT or SQLITE_ABORT_ROLLBACK may be returned, which 
will result in an OperationalError (accompanied by a nice error message 
provided by SQLite)
- for other cases, no error is returned; the operation is allowed and succeeds 
as expected
- for yet other cases, no error is returned, and the operation was rolled back

A NEWS entry should mention the change in behaviour, but I can't see how it 
would break existing projects; the current code disallows fetch across rollback 
(InterfaceError), so any problematic code would have been found, handled and 
fixed during debugging.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue44092] [sqlite3] consider removing special rollback handling

[Erlend E. Aasland]

Change by Erlend E. Aasland :


--
Removed message: https://bugs.python.org/msg393339

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue44092] [sqlite3] consider removing special rollback handling

[Erlend E. Aasland]

Change by Erlend E. Aasland :


--
Removed message: https://bugs.python.org/msg393367

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue44092] [sqlite3] consider removing special rollback handling

[Erlend E. Aasland]

Change by Erlend E. Aasland :


Removed file: https://bugs.python.org/file50029/patch.diff

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue44092] [sqlite3] consider removing special rollback handling

[Erlend E. Aasland]

Change by Erlend E. Aasland :


--
pull_requests: +24676
stage:  -> patch review
pull_request: https://github.com/python/cpython/pull/26026

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue44092] [sqlite3] consider removing special rollback handling

[Erlend E. Aasland]

Erlend E. Aasland  added the comment:

Quoting pysqlite commit 5a009ed message 
(https://github.com/ghaering/pysqlite/commit/5a009ed6fb2e90b952438f5786f93cd1e8ac8722):
"Implemented a function that resets all statements in the connection's
  statement cache. After calling this function it is always possible to
  rollback a transaction or close a connection."

The commit is from 2005-12-09. SQLite 3.7.11 wasn't released until 2012, so in 
2005 any pending statements would block a rollback. I'm guessing commit 5a009ed 
addressed that issue.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue44092] [sqlite3] consider removing special rollback handling

[Erlend E. Aasland]

Erlend E. Aasland  added the comment:

BTW, the patch also removes resetting of cursors upon close, which is a little 
bit out of scope of this bpo.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue44092] [sqlite3] consider removing special rollback handling

[Erlend E. Aasland]

Erlend E. Aasland  added the comment:

Attached patch includes the test case from bpo-33376.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue44092] [sqlite3] consider removing special rollback handling

[Erlend E. Aasland]

Change by Erlend E. Aasland :


--
keywords: +patch
Added file: https://bugs.python.org/file50029/patch.diff

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue44092] [sqlite3] consider removing special rollback handling

[Erlend E. Aasland]

New submission from Erlend E. Aasland :

Ref. bpo-33376 and bpo-10513.

Quoting from the SQLite 3.7.11 changelog[1]:
"Pending statements no longer block ROLLBACK. Instead, the pending statement 
will return SQLITE_ABORT upon next access after the ROLLBACK."

Quoting from the SQLite 3.8.7.2 changelog[2]:
"Enhance the ROLLBACK command so that pending queries are allowed to continue 
as long as the schema is unchanged. Formerly, a ROLLBACK would cause all 
pending queries to fail with an SQLITE_ABORT or SQLITE_ABORT_ROLLBACK error. 
That error is still returned if the ROLLBACK modifies the schema."

Quoting from the SQLite docs[3]:
"In more recent versions of SQLite, the ROLLBACK will proceed and pending 
statements will often be aborted, causing them to return an SQLITE_ABORT or 
SQLITE_ABORT_ROLLBACK error. In SQLite version 3.8.8 (2015-01-16) and later, a 
pending read will continue functioning after the ROLLBACK as long as the 
ROLLBACK does not modify the database schema."

I've done some tests with SQLite versions 3.35.4 and 3.7.15 where I've removed 
the call to pysqlite_do_all_statements() (introduced by [4]) in 
pysqlite_connection_rollback_impl(), and I've also removed the 
pysqlite_Cursor.reset member and all of the related code. The test suite passes 
fine (except for, as expected, the two tests that check for InterfaceError in 
case of fetch across rollback).

Do we really need to special case rollbacks anymore? I've tried to come up with 
tests that prove this approach wrong, but I haven't found any yet.


[1] https://sqlite.org/changes.html#version_3_7_11
[2] https://sqlite.org/changes.html#version_3_8_7_2
[3] https://www.sqlite.org/lang_transaction.html
[4] 
https://github.com/ghaering/pysqlite/commit/95f0956d9a78750ac8b5ca54f028b5f8d8db0abb

--
components: Extension Modules
messages: 393338
nosy: berker.peksag, erlendaasland, lemburg, serhiy.storchaka
priority: normal
severity: normal
status: open
title: [sqlite3] consider removing special rollback handling
versions: Python 3.11

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com