[issue44092] [sqlite3] Remove special rollback handling

[Pablo Galindo Salgado]

Pablo Galindo Salgado  added the comment:


New changeset a09062c267a94200ad299f779429fea1b571ee35 by Erlend Egeberg 
Aasland in branch 'main':
bpo-44092: Move What's New entry to where it belongs (GH-30381)
https://github.com/python/cpython/commit/a09062c267a94200ad299f779429fea1b571ee35


--

___
Python tracker 

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

[issue44092] [sqlite3] Remove special rollback handling

[Erlend E. Aasland]

Change by Erlend E. Aasland :


--
pull_requests: +28591
pull_request: https://github.com/python/cpython/pull/30381

___
Python tracker 

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

[issue44092] [sqlite3] Remove special rollback handling

[Erlend E. Aasland]

Change by Erlend E. Aasland :


--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

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

[issue44092] [sqlite3] Remove special rollback handling

[miss-islington]

miss-islington  added the comment:


New changeset f1a58441eea6b7788c64d03a80ea35996301e550 by Erlend Egeberg 
Aasland in branch 'main':
bpo-44092: Remove unused member `reset` from `sqlite3.Cursor` (GH-30377)
https://github.com/python/cpython/commit/f1a58441eea6b7788c64d03a80ea35996301e550


--
nosy: +miss-islington

___
Python tracker 

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

[issue44092] [sqlite3] Remove special rollback handling

[Erlend E. Aasland]

Change by Erlend E. Aasland :


--
pull_requests: +28587
stage: resolved -> patch review
pull_request: https://github.com/python/cpython/pull/30377

___
Python tracker 

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

[issue44092] [sqlite3] Remove special rollback handling

[Erlend E. Aasland]

Erlend E. Aasland  added the comment:

Reopening; there's some clean-up left.

After GH-26026, the `reset` member of `pysqlite_Cursor` is now unused. We can 
remove it and all related code. PR coming.

--
resolution: fixed -> 
status: closed -> open

___
Python tracker 

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

[issue44092] [sqlite3] Remove special rollback handling

[Pablo Galindo Salgado]

Change by Pablo Galindo Salgado :


--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

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

[issue44092] [sqlite3] Remove special rollback handling

[Pablo Galindo Salgado]

Pablo Galindo Salgado  added the comment:


New changeset 9d6a239a34a66e16188d76c23a3a770515ca44ca by Erlend Egeberg 
Aasland in branch 'main':
bpo-44092: Don't reset statements/cursors before rollback (GH-26026)
https://github.com/python/cpython/commit/9d6a239a34a66e16188d76c23a3a770515ca44ca


--
nosy: +pablogsal

___
Python tracker 

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

[issue44092] [sqlite3] Remove special rollback handling

[Ma Lin]

Ma Lin  added the comment:

If the special rollback handling is removed, the behavior of 
Connection.rollback() and 'ON CONFLICT ROLLBACK' clause will be consistent.
See attached file on_conflict_rollback.py.

--
Added file: https://bugs.python.org/file50481/on_conflict_rollback.py

___
Python tracker 

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

[issue44092] [sqlite3] Remove special rollback handling

[Erlend E. Aasland]

Erlend E. Aasland  added the comment:

> Imagine a person write a code with Python 3.11 and SQLite 3.8.7.2+, and then
> deploying it to Python 3.11 and SQLite 3.8.7.1-, error may occur. However,
> this situation is difficult to happen.

Yes, I also think this is a far fetched case. I'll see if there's an easy way 
to handle this. If there isn't, I'm not sure the added complexity will be worth 
it.

> The statement in cache will be never reused.

That is not necessarily correct; if the "problematic cursor" executes a new 
statement, the statement in the cache will be reset, thus it can be reused. 
Also, if the "problematic cursor" is closed (or deleted), the statement can 
also be reused. But, if the "problematic cursor" just gets stale and is _never_ 
used again, it may be a performance issue if it is a hot statement. But, as 
soon as I can land my changes related to the in_use flag, this will not be an 
issue anyway.

--

___
Python tracker 

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

[issue44092] [sqlite3] Remove special rollback handling

[Ma Lin]

Ma Lin  added the comment:

Imagine a person write a code with Python 3.11 and SQLite 3.8.7.2+, and then 
deploying it to Python 3.11 and SQLite 3.8.7.1-, error may occur. However, this 
situation is difficult to happen.

> Can you provide a reproducer? We've run this change through the ref.
> leak bots, and they are all green, so if there's a ref. leak, the
> test suite needs improvements.

The statement in cache will be never reused. If you don't mind, it's not a big 
problem.

--

___
Python tracker 

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

[issue44092] [sqlite3] Remove special rollback handling

[Erlend E. Aasland]

Erlend E. Aasland  added the comment:

> I mean, after this change, different versions of SQLite will behave 
> differently.

Yes, this is explained in the news item:

Fetch across rollback no longer raises :exc:`~sqlite3.InterfaceError`.
Instead we leave it to the SQLite library to handle these cases.

> And give a message for SQLITE_ABORT_ROLLBACK to explain this problem.

I'm not sure this is a good idea; I believe letting the underlying SQLite 
library explain the error is better than trying to deduce stuff based on what 
was known at compile time and what we know at runtime.

> It just leaks resource, apart from this, there seems to be no problem.

Can you provide a reproducer? We've run this change through the ref. leak bots, 
and they are all green, so if there's a ref. leak, the test suite needs 
improvements.

--

___
Python tracker 

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

[issue44092] [sqlite3] Remove special rollback handling

[Ma Lin]

Ma Lin  added the comment:

> How realistic is this scenario? If you compile with, for example 3.14.0 or
> newer, you'd link with sqlite3_trace_v2, not sqlite3_trace, so the loader
> would prevent you from running with anything pre 3.14. AFAIK, we've never
> had such problems.

I mean, after this change, different versions of SQLite will behave 
differently. And give a message for SQLITE_ABORT_ROLLBACK to explain this 
problem.

> It is a change of behaviour of the internal machinery. Does the change lead
> to wrong results (duplicate rows, wrong rows returned, no rows returned)?
> Corrupted/garbage data? Non-deterministic behaviour? Does any of the API's
> provided by sqlite3 not behave according to the documentation anymore?

It just leaks resource, apart from this, there seems to be no problem.

--

___
Python tracker 

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

[issue44092] [sqlite3] Remove special rollback handling

[Erlend E. Aasland]

Erlend E. Aasland  added the comment:

> I think this change is no problem.

Thanks, and thank you for looking reviewing this change.

> There is only one situation that a problem may occur. Write code with SQLite
> 3.8.7.2+ (2014-11-18), and run it on 3.7.15 (2012-12-12) ~ 3.8.7.1-, but
> this situation may be difficult to happen, we can note this situation in
> doc.

How realistic is this scenario? If you compile with, for example 3.14.0 or 
newer, you'd link with sqlite3_trace_v2, not sqlite3_trace, so the loader would 
prevent you from running with anything pre 3.14. AFAIK, we've never had such 
problems.

> More securely, if run on SQLite 3.8.7.1-, and encounter
> SQLITE_ABORT_ROLLBACK error code, a prompt can be given to explain the
> reason.

You already get both an error message, an (extended) error code. That should be 
sufficient.

> Also note that the current main branch is buggy. If don't adopt this change
> or revert this change later, don't forget to fix the bug of msg407185
> (`pysqlite_Statement.in_use` flag is not reset).

It is a change of behaviour of the internal machinery. Does the change lead to 
wrong results (duplicate rows, wrong rows returned, no rows returned)? 
Corrupted/garbage data? Non-deterministic behaviour? Does any of the API's 
provided by sqlite3 not behave according to the documentation anymore?

NB: I plan to get rid of the in_use flag long before the beta sets in.

--

___
Python tracker 

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

[issue44092] [sqlite3] Remove special rollback handling

[Ma Lin]

Ma Lin  added the comment:

I think this change is no problem.
Erlend E. Aasland's explanation is very clear. 

There is only one situation that a problem may occur. Write code with SQLite 
3.8.7.2+ (2014-11-18), and run it on 3.7.15 (2012-12-12) ~ 3.8.7.1-, but this 
situation may be difficult to happen, we can note this situation in doc.

More securely, if run on SQLite 3.8.7.1-, and encounter SQLITE_ABORT_ROLLBACK 
error code, a prompt can be given to explain the reason.

Also note that the current main branch is buggy. If don't adopt this change or 
revert this change later, don't forget to fix the bug of msg407185 
(`pysqlite_Statement.in_use` flag is not reset).

--
nosy: +malin

___
Python tracker 

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

[issue44092] [sqlite3] Remove special rollback handling

[Erlend E. Aasland]

Erlend E. Aasland  added the comment:

Berker, does this look ok to you?

--
title: [sqlite3] consider removing special rollback handling -> [sqlite3] 
Remove special rollback handling

___
Python tracker 

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