[issue27881] Fix possible bugs when setting sqlite3.Connection.isolation_level

Berker Peksag added the comment: Closing this as 'fixed'. Any further improvement can be discussed in separate issues. Thanks! -- resolution: -> fixed stage: patch review -> resolved status: open -> closed ___ Python tracker

[issue27881] Fix possible bugs when setting sqlite3.Connection.isolation_level

Roundup Robot added the comment: New changeset 546b1f70cbed by Serhiy Storchaka in branch '3.5': Issue #27881: Fixed possible bugs when setting sqlite3.Connection.isolation_level. https://hg.python.org/cpython/rev/546b1f70cbed New changeset 96e05f1af2c8 by Serhiy Storchaka in branch 'default':

[issue27881] Fix possible bugs when setting sqlite3.Connection.isolation_level

Aviv Palivoda added the comment: The only change I see that can happen is that we return upper case isolation level when the user used a lower case. I think that it is worth to slow down the getter for the code simplification. -- ___ Python tracker

[issue27881] Fix possible bugs when setting sqlite3.Connection.isolation_level

Serhiy Storchaka added the comment: Yes, I thought about this. This changes the behavior (for now isolation_level returns exactly the same object that was assigned to it) and slows down the getter. This can be added only on the default branch. -- __

[issue27881] Fix possible bugs when setting sqlite3.Connection.isolation_level

Aviv Palivoda added the comment: What do you think about removing the isolation_level member from the pysqlite_Connection. It is only there to be for the pysqlite_connection_get_isolation_level and that could be easily calculated from the begin_statement. -- Added file: http://bugs.py

[issue27881] Fix possible bugs when setting sqlite3.Connection.isolation_level

Xiang Zhang added the comment: Hmm, you do this "It's going to be freed in the dealloc method unless your alter that part too". If this is done I admit this is more clean. Patch LGTM. -- ___ Python tracker ___

[issue27881] Fix possible bugs when setting sqlite3.Connection.isolation_level

Serhiy Storchaka added the comment: > Xiang what do you think about changing the isolation_levels list to > begin_statements list This is interesting suggestion. Indeed, this simplifies the code. Thanks Aviv. -- Added file: http://bugs.python.org/file44318/issue27881_v4.patch

[issue27881] Fix possible bugs when setting sqlite3.Connection.isolation_level

Xiang Zhang added the comment: I thought about this method but didn't find it's simpler. You cannot avoid malloc/free since the isolation level has to be on heap. It's going to be freed in the dealloc method unless your alter that part too. Then it's about one memcpy or two, which doesn't make

[issue27881] Fix possible bugs when setting sqlite3.Connection.isolation_level

Aviv Palivoda added the comment: Xiang what do you think about changing the isolation_levels list to begin_statements list: static const char * const begin_statements[] = {"BEGIN", "BEGIN DEFERRED", "BEGIN IMMEDIATE", "BEGIN EXCLUSIVE", NULL}; This change will allow you to remove all the strin

[issue27881] Fix possible bugs when setting sqlite3.Connection.isolation_level

Xiang Zhang added the comment: Leave replies and make a trival change in v3. I don't change the other two just as I state in the replies. But please do what you like. -- Added file: https://bugs.python.org/file44266/issue27881_v3.patch ___ Python tra

[issue27881] Fix possible bugs when setting sqlite3.Connection.isolation_level

Serhiy Storchaka added the comment: In general LGTM. But added several style comments. -- ___ Python tracker ___ ___ Python-bugs-list

[issue27881] Fix possible bugs when setting sqlite3.Connection.isolation_level

Xiang Zhang added the comment: Serhiy, I've updated the patch. Looking forward to your feedback. -- Added file: https://bugs.python.org/file44265/issue27881_v2.patch ___ Python tracker

[issue27881] Fix possible bugs when setting sqlite3.Connection.isolation_level

Changes by Serhiy Storchaka : -- assignee: -> serhiy.storchaka stage: -> patch review ___ Python tracker ___ ___ Python-bugs-list m

[issue27881] Fix possible bugs when setting sqlite3.Connection.isolation_level

Xiang Zhang added the comment: issue27881.patch tires to solve this. Hope to get feedback. -- keywords: +patch Added file: https://bugs.python.org/file44247/issue27881.patch ___ Python tracker

[issue27881] Fix possible bugs when setting sqlite3.Connection.isolation_level

New submission from Xiang Zhang: This is a follow up of issue27861 and means to fix the second issue mentioned in it. pysqlite_connection_set_isolation_level now does not check allowed values and when any part fails, it leaves the Connection object in an inconsistent state. I'll write a patch