[Django] #30953: Model Inheritance and select_for

Django

unread,

Nov 5, 2019, 3:50:14 AM11/5/19

to django-...@googlegroups.com

#30953: Model Inheritance and select_for_update with 'of'
-------------------------------------+-------------------------------------
Reporter: Abhijeet | Owner: nobody
Type: Bug | Status: new
Component: Database | Version: 2.2
layer (models, ORM) | Keywords: race conditions,
Severity: Normal | psql, concurrency, db
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
Consider two models A and B with B being a sub model of A. If I have a
select and update that looks like this (assume it all happens inside an
atomic transaction):

{{{
b = B.objects.filter(pk=1).select_for_update(of=('self'))
b = b[0] # Evaluate the query
b.some_field_of_b = b.some_field_of_b + 1
b.save()
}}}

This will cause race conditions. The generated SQL will only lock the
table of A (the super-model) and not of B. As you can see, the field being
updated is of B and not of A. Even though the `SELECT .. FOR UPDATE OF`
might block, the queryset returned will not contain the updated data of
the table of B because the snapshot will not include the updated data
(before a lock is even acquired).

When the `of` parameter for `select_for_update` is supplied, the tables
locked should include those of the sub-models of the `of` fields.

--
Ticket URL: <https://code.djangoproject.com/ticket/30953>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,

Nov 5, 2019, 4:02:15 AM11/5/19

to django-...@googlegroups.com

#30953: Model Inheritance and select_for_update with 'of'
-------------------------------------+-------------------------------------

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Abhijeet):

* owner: nobody => Abhijeet
* status: new => assigned

--
Ticket URL: <https://code.djangoproject.com/ticket/30953#comment:1>

Django

unread,

Nov 5, 2019, 12:21:40 PM11/5/19

to django-...@googlegroups.com

#30953: Model Inheritance and select_for_update with 'of'
-------------------------------------+-------------------------------------

Reporter: Abhijeet | Owner: Abhijeet
Type: Bug | Status: assigned

Component: Database layer | Version: master

(models, ORM) |
Severity: Normal | Resolution:

Keywords: select_for_update | Triage Stage: Accepted
mti model inheritance |
Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Simon Charette):

* keywords: race conditions, psql, concurrency, db => select_for_update
mti model inheritance
* version: 2.2 => master
* stage: Unreviewed => Accepted

Comment:

> The generated SQL will only lock the table of A (the super-model) and
not of B.

I confirmed this is the case, `self` always use the parent model table
instead of the current one.

I'm not convinced we should make `select_for_update(of=('self',))` select
'''all''' inherited tables though as that would prevents selecting only
the current ''leaf'' model table. IMO the code should only be changed to
make `self` point to the queryset's model table as documented.

I suggest we also enhance the documentation to mention that parent links
should be included in `of` when this is desired instead.

In your case that would mean `of=('self', 'a_ptr')` to select both tables
for update.

--
Ticket URL: <https://code.djangoproject.com/ticket/30953#comment:2>

Django

unread,

Nov 17, 2019, 2:33:03 AM11/17/19

to django-...@googlegroups.com

#30953: Model Inheritance and select_for_update with 'of'
-------------------------------------+-------------------------------------

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Abhijeet):

* cc: Abhijeet (added)
* has_patch: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/30953#comment:3>

Django

unread,

Nov 17, 2019, 2:50:03 AM11/17/19

to django-...@googlegroups.com

#30953: Model Inheritance and select_for_update with 'of'
-------------------------------------+-------------------------------------

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Abhijeet):

Created a patch pull request on GitHub. ([PR
#12082](https://github.com/django/django/pull/12082)).

I believe this bug qualifies under data loss bugs as the unexpected
behavior caused due to this bug could cause race conditions (as was my
case). I've added release notes to 2.2.8. However, I'm not sure if it
should be added to 3.0 or 3.1 and should be backported to 2.1. Please
guide me on this so that I can modify the patch appropriately.

--
Ticket URL: <https://code.djangoproject.com/ticket/30953#comment:4>

Django

unread,

Nov 19, 2019, 6:32:34 AM11/19/19

to django-...@googlegroups.com

#30953: Model Inheritance and select_for_update with 'of'
-------------------------------------+-------------------------------------

Reporter: Abhijeet | Owner: Abhijeet
Type: Bug | Status: assigned

Component: Database layer | Version: 2.1
(models, ORM) |
Severity: Release blocker | Resolution:

Keywords: select_for_update | Triage Stage: Accepted
mti model inheritance |

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by felixxm):

* version: master => 2.1
* severity: Normal => Release blocker

Comment:

I agree this qualifies for a backport as a data loss bug.

--
Ticket URL: <https://code.djangoproject.com/ticket/30953#comment:5>

Django

unread,

Nov 22, 2019, 9:35:40 AM11/22/19

to django-...@googlegroups.com

#30953: Model Inheritance and select_for_update with 'of'
-------------------------------------+-------------------------------------

Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by felixxm):

* needs_better_patch: 0 => 1
* needs_tests: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/30953#comment:6>

Django

unread,

Nov 29, 2019, 4:46:57 AM11/29/19

to django-...@googlegroups.com

#30953: Model Inheritance and select_for_update with 'of'
-------------------------------------+-------------------------------------

Reporter: Abhijeet | Owner: felixxm

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by felixxm):

* owner: Abhijeet => felixxm
* needs_better_patch: 1 => 0
* needs_tests: 1 => 0

Comment:

[https://github.com/django/django/pull/12154 PR]

--
Ticket URL: <https://code.djangoproject.com/ticket/30953#comment:7>

Django

unread,

Dec 2, 2019, 1:58:59 AM12/2/19

to django-...@googlegroups.com

#30953: Model Inheritance and select_for_update with 'of'
-------------------------------------+-------------------------------------

Reporter: Abhijeet | Owner: felixxm
Type: Bug | Status: closed

Component: Database layer | Version: 2.1
(models, ORM) |

Severity: Release blocker | Resolution: fixed

Keywords: select_for_update | Triage Stage: Accepted
mti model inheritance |

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by GitHub <noreply@…>):

* status: assigned => closed
* resolution: => fixed

Comment:

In [changeset:"0107e3d1058f653f66032f7fd3a0bd61e96bf782" 0107e3d1]:
{{{
#!CommitTicketReference repository=""
revision="0107e3d1058f653f66032f7fd3a0bd61e96bf782"
Fixed #30953 -- Made select_for_update() lock queryset's model when using
"self" with multi-table inheritance.

Thanks Abhijeet Viswa for the report and initial patch.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/30953#comment:8>

Django

unread,

Dec 2, 2019, 1:59:20 AM12/2/19

to django-...@googlegroups.com

#30953: Model Inheritance and select_for_update with 'of'
-------------------------------------+-------------------------------------

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Mariusz Felisiak <felisiak.mariusz@…>):

In [changeset:"f4ed6800bd5de576816861931699ddd8377d338d" f4ed6800]:
{{{
#!CommitTicketReference repository=""
revision="f4ed6800bd5de576816861931699ddd8377d338d"
[3.0.x] Fixed #30953 -- Made select_for_update() lock queryset's model

when using "self" with multi-table inheritance.

Thanks Abhijeet Viswa for the report and initial patch.

Backport of 0107e3d1058f653f66032f7fd3a0bd61e96bf782 from master
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/30953#comment:9>

Django

unread,

Dec 2, 2019, 2:04:09 AM12/2/19

to django-...@googlegroups.com

#30953: Model Inheritance and select_for_update with 'of'
-------------------------------------+-------------------------------------

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Mariusz Felisiak <felisiak.mariusz@…>):

In [changeset:"6cf3b6f5cf0cc3b11e86e511ec5201999913286f" 6cf3b6f5]:
{{{
#!CommitTicketReference repository=""
revision="6cf3b6f5cf0cc3b11e86e511ec5201999913286f"
[2.2.x] Fixed #30953 -- Made select_for_update() lock queryset's model

when using "self" with multi-table inheritance.

Thanks Abhijeet Viswa for the report and initial patch.
Backport of 0107e3d1058f653f66032f7fd3a0bd61e96bf782 from master
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/30953#comment:10>

Django

unread,

Dec 2, 2019, 2:12:40 AM12/2/19

to django-...@googlegroups.com

#30953: Model Inheritance and select_for_update with 'of'
-------------------------------------+-------------------------------------

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Mariusz Felisiak <felisiak.mariusz@…>):

In [changeset:"39e39d0ac1b720e7460ec8ccf45926c78edb2047" 39e39d0a]:
{{{
#!CommitTicketReference repository=""
revision="39e39d0ac1b720e7460ec8ccf45926c78edb2047"
Refs #30953 -- Added 2.1.15 release note for
0107e3d1058f653f66032f7fd3a0bd61e96bf782.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/30953#comment:11>

Django

unread,

Dec 2, 2019, 2:13:07 AM12/2/19

to django-...@googlegroups.com

#30953: Model Inheritance and select_for_update with 'of'
-------------------------------------+-------------------------------------

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Mariusz Felisiak <felisiak.mariusz@…>):

In [changeset:"db0cc4ae96c4752d10d98a3c7f2c48f813bf8a7f" db0cc4a]:
{{{
#!CommitTicketReference repository=""
revision="db0cc4ae96c4752d10d98a3c7f2c48f813bf8a7f"
[3.0.x] Refs #30953 -- Added 2.1.15 release note for
0107e3d1058f653f66032f7fd3a0bd61e96bf782.

Backport of 39e39d0ac1b720e7460ec8ccf45926c78edb2047 from master
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/30953#comment:12>

Django

unread,

Dec 2, 2019, 2:13:29 AM12/2/19

to django-...@googlegroups.com

#30953: Model Inheritance and select_for_update with 'of'
-------------------------------------+-------------------------------------

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Mariusz Felisiak <felisiak.mariusz@…>):

In [changeset:"70311e1d00ef5b6bbbc8961eac81b5c814396a43" 70311e1]:
{{{
#!CommitTicketReference repository=""
revision="70311e1d00ef5b6bbbc8961eac81b5c814396a43"
[2.2.x] Refs #30953 -- Added 2.1.15 release note for
0107e3d1058f653f66032f7fd3a0bd61e96bf782.

Backport of 39e39d0ac1b720e7460ec8ccf45926c78edb2047 from master
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/30953#comment:13>

Django

unread,

Dec 2, 2019, 2:20:25 AM12/2/19

to django-...@googlegroups.com

#30953: Model Inheritance and select_for_update with 'of'
-------------------------------------+-------------------------------------

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Mariusz Felisiak <felisiak.mariusz@…>):

In [changeset:"f57f81a7fe9da5116a9892d7f32aa055771b99c0" f57f81a7]:
{{{
#!CommitTicketReference repository=""
revision="f57f81a7fe9da5116a9892d7f32aa055771b99c0"
[2.1.x] Refs #30953 -- Added 2.1.15 release note for
0107e3d1058f653f66032f7fd3a0bd61e96bf782.

Backport of 39e39d0ac1b720e7460ec8ccf45926c78edb2047 from master
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/30953#comment:15>

Django

unread,

Dec 2, 2019, 2:20:25 AM12/2/19

to django-...@googlegroups.com

#30953: Model Inheritance and select_for_update with 'of'
-------------------------------------+-------------------------------------

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Mariusz Felisiak <felisiak.mariusz@…>):

In [changeset:"015fab76ad7f91562d2323657a6b5d7b4ff15e28" 015fab76]:
{{{
#!CommitTicketReference repository=""
revision="015fab76ad7f91562d2323657a6b5d7b4ff15e28"
[2.1.x] Fixed #30953 -- Made select_for_update() lock queryset's model

[Django] #30953: Model Inheritance and select_for_update with 'of'

Django

Django

Django

Django

Django

Django

Django

Django

Django

Django

Django

Django

Django

Django

Django

Django