zope.sqlalchemy icon indicating copy to clipboard operation
zope.sqlalchemy copied to clipboard

SQLite do support SAVEPOINT

Open jimmy-lt opened this issue 5 years ago • 1 comments

Following SQLAchemy's documentation, savepoint with SQLite is supported (excluding some minro issues with the pysqlite driver where a workaround is avaible).

When trying to use SAVEPOINT with SQLite, the following error is raised:

Traceback (most recent call last):
  File "/lib/python3.7/site-packages/transaction/_transaction.py", line 631, in __init__
    savepoint = datamanager.savepoint
  File "/lib/python3.7/site-packages/zope/sqlalchemy/datamanager.py", line 155, in savepoint
    raise AttributeError("savepoint")
AttributeError: savepoint

This is due because SQLite is explicitly declared as not supported:

https://github.com/zopefoundation/zope.sqlalchemy/blob/ae4745cd85f5b04445929e8c1761e1a60e02a441/src/zope/sqlalchemy/datamanager.py#L59 https://github.com/zopefoundation/zope.sqlalchemy/blob/ae4745cd85f5b04445929e8c1761e1a60e02a441/src/zope/sqlalchemy/datamanager.py#L150-L155

However, if I remove sqlite from the set, my transaction is acting as expected:

[2020-03-30 21:01:12 - INFO/sqlalchemy.engine.base.Engine:711] BEGIN (implicit)
[2020-03-30 21:01:12 - INFO/sqlalchemy.engine.base.Engine:1205] SELECT xxx
[2020-03-30 21:01:12 - INFO/sqlalchemy.engine.base.Engine:1210] ()
[2020-03-30 21:01:12 - INFO/sqlalchemy.engine.base.Engine:1205] SAVEPOINT sa_savepoint_1
[2020-03-30 21:01:12 - INFO/sqlalchemy.engine.base.Engine:1210] ()
[2020-03-30 21:01:12 - INFO/sqlalchemy.engine.base.Engine:1205] INSERT INTO xxx
[2020-03-30 21:01:12 - INFO/sqlalchemy.engine.base.Engine:1210] ()
[2020-03-30 21:01:12 - INFO/sqlalchemy.engine.base.Engine:1205] ROLLBACK TO SAVEPOINT sa_savepoint_1
[2020-03-30 21:01:12 - INFO/sqlalchemy.engine.base.Engine:1210] ()
[2020-03-30 21:01:12 - INFO/sqlalchemy.engine.base.Engine:1205] SELECT xxx

My environment:

SQLAlchemy==1.3.15
transaction==3.0.0
zope.sqlalchemy==1.3

jimmy-lt avatar Mar 30 '20 19:03 jimmy-lt

In retrospect it would have been better to add a savepoints=True parameter to register rather than try to guess based on database drivername. As noted by ajung in that file, support varies by database version and sqlite savepoints now work. I think it would make sense to fix this.

lrowe avatar Mar 30 '20 19:03 lrowe