[Django] #25955: INITIALLY DEFERRED, nested transaction.atomic, postgresql
Django
-------------------------------------+-------------------------------------
Reporter: aklim007 | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) | Keywords: DEFERRED
Severity: Normal | transaction.atomic savepoint
| postgresql
Triage Stage: Unreviewed | Has patch: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
{{{
#!div style="font-size: 80%"
Create 2 models:
{{{#!python
class Model1(models.Model):
pass
class Model2(models.Model):
model1 = models.ForeignKey(Model1)
}}}
}}}
{{{
#!div style="font-size: 80%"
foreign key in DB:
{{{#!postgresql
CONSTRAINT zabbix_model2_model1_id_4d6fe5808c3891b4_fk_zabbix_model1_id
FOREIGN KEY (model1_id)
REFERENCES zabbix_model1 (id) MATCH SIMPLE
ON UPDATE NO ACTION ON DELETE NO ACTION DEFERRABLE INITIALLY
DEFERRED
-- INITIALLY DEFERRED !!!!!!!!!!!!!!!!!!!
-- DEFERRABLE constraints set to DEFERRED (INITIALLY DEFERRED or via SET
CONSTRAINTS) are checked after each transaction (not savepoint release).
}}}
}}}
{{{
#!div style="font-size: 80%"
add row in model1:
{{{#!python
>>> Model1().save()
>>> Model1.objects.values()
[{'id': 1}]
}}}
}}}
{{{
#!div style="font-size: 80%"
view:
{{{#!python
@transaction.atomic()
def _test(request):
model1_ids = [
1,
2, # incorrect ForeignKey
1.
]
for model1_id in model1_ids:
try:
with transaction.atomic():
Model2(model1_id=model1_id).save()
except IntegrityError as e:
# magic
pass
return render(request, 'test_template.html')
}}}
}}}
> Request Method: GET
> Request URL: http://127.0.0.1:8000/test
> Django Version: 1.8.6
> Exception Type: IntegrityError
> Exception Value:
> insert or update on table "zabbix_model2" violates foreign key
constraint "zabbix_model2_model1_id_4d6fe5808c3891b4_fk_zabbix_model1_id"
> DETAIL: Key (model1_id)=(2) is not present in table "zabbix_model1".
> Exception Location: /home/its/venv/lib/python3.4/site-
packages/django/db/backends/base/base.py in _commit, line 142
> Python Executable: /home/its/venv/bin/python
> Python Version: 3.4.2
[[Image(http://i5.imageban.ru/out/2015/12/20/58dc28d049b669a5633a8a7b9643234b.png)]]
--
Ticket URL: <https://code.djangoproject.com/ticket/25955>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: aklim007 | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: DEFERRED transaction.atomic | Triage Stage:
savepoint postgresql | Unreviewed
Has patch: 1 | Easy pickings: 0
UI/UX: 0 |
Changes (by aklim007):
* Attachment "django_master.patch" added.
Django
Reporter: aklim007 | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: DEFERRED transaction.atomic | Triage Stage:
Has patch: 1 | Easy pickings: 0
UI/UX: 0 |
Changes (by aklim007):
* Attachment "django_stable_1.8.patch" added.
Django
Reporter: aklim007 | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: DEFERRED transaction.atomic | Triage Stage:
Has patch: 1 | Easy pickings: 0
UI/UX: 0 |
Changes (by aklim007):
* Attachment "django_stable_1.9.patch" added.
Django
-------------------------------------+-------------------------------------
Reporter: aklim007 | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) |
Keywords: DEFERRED | Triage Stage:
transaction.atomic savepoint | Unreviewed
postgresql |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
Comment:
This is the expected behavior, as far as I can see. Can you elaborate on
what you expected to happen?
--
Ticket URL: <https://code.djangoproject.com/ticket/25955#comment:1>
Django
-------------------------------------+-------------------------------------
Reporter: aklim007 | Owner: nobody
Component: Database layer | Version: master
(models, ORM) |
Keywords: DEFERRED | Triage Stage:
transaction.atomic savepoint | Unreviewed
postgresql |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
* status: new => closed
* resolution: => wontfix
Comment:
Oh, looking at the patch, I see you expect deferred constraints to be
checked at the end of nested `atomic` blocks.
Unless you can point some place in the documentation where such behavior
is described, I'd call this a huge backward incompatibility. If you can
show such a place, please re-open.
--
Ticket URL: <https://code.djangoproject.com/ticket/25955#comment:2>
Django
Reporter: aklim007 | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) |
Keywords: DEFERRED | Triage Stage:
transaction.atomic savepoint | Unreviewed
postgresql |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
* cc: aklim007@… (added)
* status: closed => new
* resolution: wontfix =>
Comment:
Replying to [comment:2 shaib]:
> Oh, looking at the patch, I see you expect deferred constraints to be
checked at the end of nested `atomic` blocks.
>
> Unless you can point some place in the documentation where such behavior
is described, I'd call this a huge backward incompatibility. If you can
show such a place, please re-open.
https://docs.djangoproject.com/en/1.9/topics/db/transactions/#controlling-
transactions-explicitly
{{{
#!div style="font-size: 80%"
Code highlighting:
{{{#!python
from django.db import IntegrityError, transaction
@transaction.atomic
def viewfunc(request):
create_parent()
try:
with transaction.atomic():
generate_relationships()
except IntegrityError:
handle_exception()
add_children()
}}}
}}}
In mysql it works fine.
The current behavior of Postgres violates the logic transaktion.atomic.
To work correctly, I have to think about nesting transaktion.atomic.
If the function is wrapped in a atomic, atomic cause within another unit
will conduct one another more.
If the unit is completed successfully, I expect that everything is in
order and there is atomic.
--
Ticket URL: <https://code.djangoproject.com/ticket/25955#comment:3>
Django
Reporter: aklim007 | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) |
Keywords: DEFERRED | Triage Stage:
transaction.atomic savepoint | Unreviewed
postgresql |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Comment (by shaib):
The docs you cite say very explicitly what Django does with transactions
and savepoints, and the behavior is in complete accordance. They do imply
what you took from them, and the point that the documentation's example
doesn't really work under postgresql is strong. However, forcing
constraint checks at the end of every nested atomic block is likely to
break a lot of existing code, so the fix you suggested cannot be accepted;
not for master, and most certainly not for the released branches.
We can treat this two ways: One is as a behavior problem; to fix it as
such, you'll need to provide an upgrade path -- that is, a method for
users to find out that they have a problem and fix it or work around it.
Also, no change in transactional behavior is likely to be accepted without
a discussion on the DevelopersMailingList, so if you want to see this kind
of change happen, please write to the list.
The other is as a documentation problem: Make the example create integrity
errors that are not about foreign keys, and perhaps add a paragraph
explaining that deferred constraints are not necessarily checked at the
end of atomic blocks.
--
Ticket URL: <https://code.djangoproject.com/ticket/25955#comment:4>
Django
Reporter: aklim007 | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) |
Keywords: DEFERRED | Triage Stage:
transaction.atomic savepoint | Unreviewed
postgresql |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Comment (by aklim007):
How about adding a parameter in OPTIONS?
{{{
#!div style="font-size: 80%"
Code highlighting:
{{{#!python
DATABASES = {
'default': {
'ENGINE': 'django.db.backends.postgresql_psycopg2',
# ...
'OPTIONS': {
'savepoint_check_constraints': True
}
},
}
}}}
full backward compatibility
add patch
--
Ticket URL: <https://code.djangoproject.com/ticket/25955#comment:5>
Django
Reporter: aklim007 | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) |
Keywords: DEFERRED | Triage Stage:
transaction.atomic savepoint | Unreviewed
postgresql |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
* Attachment "django_master_v2.patch" added.
Django
Reporter: aklim007 | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) |
Keywords: DEFERRED | Triage Stage:
transaction.atomic savepoint | Unreviewed
postgresql |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Comment (by shaib):
As I said before, changes in transactional behavior will not happen
without discussion on the DevelopersMailingList.
Also, you may benefit from reading
https://docs.djangoproject.com/en/stable/intro/contributing/ and
https://docs.djangoproject.com/en/stable/internals/contributing/writing-
code/
--
Ticket URL: <https://code.djangoproject.com/ticket/25955#comment:6>
Django
Reporter: aklim007 | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) |
Keywords: DEFERRED | Triage Stage:
postgresql |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
* stage: Unreviewed => Someday/Maybe
Comment:
Bumping to "Someday/Maybe" pending the outcome of the
[https://groups.google.com/d/topic/django-
developers/7YzI3VWmiHY/discussion mailing list discussion].
--
Ticket URL: <https://code.djangoproject.com/ticket/25955#comment:7>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) |
Keywords: DEFERRED | Triage Stage:
transaction.atomic savepoint | Someday/Maybe
postgresql |
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/25955#comment:8>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: dev
Severity: Normal | Resolution:
Keywords: DEFERRED | Triage Stage:
transaction.atomic savepoint | Someday/Maybe
postgresql |
Has patch: 1 | Needs documentation: 0
-------------------------------------+-------------------------------------
* cc: Ülgen Sarıkavak (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/25955#comment:9>