[Django] #32527: needs_rollback flag issue with implementation of backend that does not support savepoint
Django
support savepoint
-------------------------------------+-------------------------------------
Reporter: Hemant | Owner: nobody
Bhanawat |
Type: Bug | Status: new
Component: Database | Version: dev
layer (models, ORM) | Keywords: savepoint
Severity: Normal | needs_rollback database backend
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
I am implementing a backend for Yugabyte. Yugabyte currently doesn't
support savepoints. Hence, in the backend, I have set uses_savepoints=
False
However, while running the test. I hit the following issue.
The error: "TransactionManagementError: An error occurred in the current
transaction. You can't execute queries until the end of the 'atomic'
block."
On some investigation, I found that this exception is thrown if
self.needs_rollback is True. The first test that uses savepoint passes
successfully. However, the second test has needs_rollback set as true and
hits this exception.
The problem stems from the following code of __enter__ function in
transaction.py
if self.savepoint and not connection.needs_rollback:
sid = connection.savepoint()
connection.savepoint_ids.append(sid)
else:
connection.savepoint_ids.append(None)
Basically, a None is inserted in the savepoint_ids when the savepoints are
not supported. And the following code in __exit__ function of
transaction.py, it sets needs_rollback as true:
if sid is None:
connection.needs_rollback = True
Can someone please clarify if this is an issue with the code or there is
something else that we can do at our layer. For the time being, we have
overridden the function savepoint of DatabaseWrapper to return 1. And, we
ask the users of our backend to not set savepoint=true in parameters of
transaction.atomic().
def savepoint(self):
return 1
--
Ticket URL: <https://code.djangoproject.com/ticket/32527>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
support savepoint
-------------------------------------+-------------------------------------
Type: Bug | Status: closed
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution: needsinfo
Keywords: savepoint | Triage Stage:
needs_rollback database backend | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => closed
* resolution: => needsinfo
Old description:
New description:
--
Comment:
Thanks for this report, however I'm not able to confirm (with the given
details) that it is an issue in Django. As far as I'm aware it should work
properly. In Django < 2.0 we supported SQLite < 3.7.5 with the same
configuration (see 27193aea0088b238e3ee0f0f235364a34a09265c) and it worked
fine. You can try to create a topic on the https://forum.djangoproject.com
or a discussion on DevelopersMailingList.
Feel-free to reopen the ticket when you can provide more details and prove
it's an issue in Django.
--
Ticket URL: <https://code.djangoproject.com/ticket/32527#comment:1>
Django
support savepoint
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
needs_rollback database backend | Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: closed => new
* resolution: needsinfo =>
Comment:
Before Django 2.0, sql server had different handling.
{{{
if not connection.get_autocommit():
# Some database adapters (namely sqlite3) don't handle
# transactions and savepoints properly when autocommit is
off.
# Turning autocommit back on isn't an option; it would
trigger
# a premature commit. Give up if that happens.
if connection.features.autocommits_when_autocommit_is_off:
raise TransactionManagementError(
"Your database backend doesn't behave properly
when "
"autocommit is off. Turn it on before using
'atomic'.")
}}}
Please create a dummy backend with the following two settings or may be
try to change an existing one and add these two settings and you will hit
this issue.
{{{
uses_savepoints= False
can_release_savepoints = False
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32527#comment:2>
Django
support savepoint
-------------------------------------+-------------------------------------
Component: Database layer | Version: dev
(models, ORM) |
Keywords: savepoint | Triage Stage:
needs_rollback database backend | Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => closed
* resolution: => needsinfo
Comment:
You didn't mention `autocommit` before, I have a feeling that more bits
are missing here. Can you provide a small project illustrating this issue?
--
Ticket URL: <https://code.djangoproject.com/ticket/32527#comment:3>
Django
support savepoint
-------------------------------------+-------------------------------------
Type: Bug | Status: closed
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution: needsinfo
Keywords: savepoint | Triage Stage:
needs_rollback database backend | Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Hemant Bhanawat):
On master branch of django, apply this commit:
{{{
diff --git a/django/db/backends/postgresql/features.py
b/django/db/backends/postgresql/features.py
index 722bfe0475..7365b57b54 100644
--- a/django/db/backends/postgresql/features.py
+++ b/django/db/backends/postgresql/features.py
@@ -6,6 +6,8 @@ from django.utils.functional import cached_property
class DatabaseFeatures(BaseDatabaseFeatures):
+ uses_savepoints= False
+ can_release_savepoints = False
allows_group_by_selected_pks = True
can_return_columns_from_insert = True
can_return_rows_from_bulk_insert = True
}}}
The run the following command to run the tests:
{{{
./runtests.py --settings=postgres_settings -v 3 admin_changelist
--failfast
}}}
The second test will fail because needs_rollback is set as true.
--
Ticket URL: <https://code.djangoproject.com/ticket/32527#comment:4>
Django
support savepoint
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
needs_rollback database backend | Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: closed => new
* resolution: needsinfo =>
--
Ticket URL: <https://code.djangoproject.com/ticket/32527#comment:5>
Django
support savepoint
-------------------------------------+-------------------------------------
Component: Database layer | Version: dev
(models, ORM) |
Keywords: savepoint | Triage Stage:
needs_rollback database backend | Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => closed
* resolution: => duplicate
Comment:
Duplicate of #28263.
--
Ticket URL: <https://code.djangoproject.com/ticket/32527#comment:6>