search

zopefoundation / zope.sqlalchemy

Integration of SQLAlchemy with transaction management
Other
32 stars 34 forks source link

SQLite do support SAVEPOINT #44

Open jimmy-lt opened 4 years ago

jimmy-lt commented 4 years ago

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
lrowe commented 4 years ago

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.