[Django] #30129: Request to Allow F() Expressions in Subquery() on Insert
Django
-------------------------------------+-------------------------------------
Reporter: Charlie | Owner: nobody
McBride |
Type: New | Status: new
feature |
Component: Database | Version: 2.1
layer (models, ORM) | Keywords: Subquery, F, Query
Severity: Normal | Expressions
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 1
UI/UX: 0 |
-------------------------------------+-------------------------------------
I understand why F() expressions are generally disallowed inside insert
statements, the columns you are referencing don't yet exist, so it
wouldn't make any sense to do so. However, if you are performing an insert
with a Subquery (as in the example below), it's possible to have otherwise
valid statements rejected because of the blanket blacklist of F()
expressions during inserts.
For example:
given the following models:
{{{
class Item(models.Model):
pass
class ItemVersion(models.Model):
item = models.ForeignKey(Item, related_name='versions')
version_number = models.IntegerField(default=0)
class Meta:
unique_together = ('item', 'version_number',)
}}}
I would like to be able to do the following operation to mitigate race
conditions as described here
(https://docs.djangoproject.com/en/2.1/ref/models/expressions/#avoiding-
race-conditions-using-f)
{{{
item = Item.objects.create()
# arbitrary number of other items created/destroyed etc
item_version_2 = ItemVersion.objects.create(
item=item,
version_number=Subquery(
item.versions.order_by('- version_number').annotate(
max_version_number=Coalesce(Max('version_number'), 0)
).annotate(
new_version_number=F('max_version_number) + 1
).values('new_version_number')[:1]
)
)
}}}
As written, I would expect the F() in the inner expression to always be
resolvable, because it is in a Subquery (and not the result of an insert).
However this query is blocked by the compiler because "F() expressions can
only be used to update, not to insert." Would it be possible to allow F
expressions in a Subquery even if it is being used in an insert? Is there
an edge case that I'm missing that caused the team to not consider this?
--
Ticket URL: <https://code.djangoproject.com/ticket/30129>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
expressions
-------------------------------------+-------------------------------------
Reporter: Charlie McBride | Owner: nobody
Type: New feature | Status: new
Component: Database layer | Version: 2.1
(models, ORM) |
Severity: Normal | Resolution:
Keywords: Subquery, F, Query | Triage Stage:
Expressions | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Changes (by Tim Graham):
* easy: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/30129#comment:1>
Django
expressions
-------------------------------------+-------------------------------------
Reporter: Charlie McBride | Owner: nobody
Type: New feature | Status: new
Component: Database layer | Version: 2.1
(models, ORM) |
Severity: Normal | Resolution:
Keywords: Subquery, F, Query | Triage Stage:
Expressions | Unreviewed
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Description changed by Charlie McBride:
Old description:
New description:
I understand why F() expressions are generally disallowed inside insert
statements, the columns you are referencing don't yet exist, so it
wouldn't make any sense to do so. However, if you are performing an insert
with a Subquery (as in the example below), it's possible to have otherwise
valid statements rejected because of the blanket blacklist of F()
expressions during inserts.
For example:
given the following models:
{{{
class Item(models.Model):
pass
class ItemVersion(models.Model):
item = models.ForeignKey(Item, related_name='versions')
version_number = models.IntegerField(default=0)
class Meta:
unique_together = ('item', 'version_number',)
}}}
I would like to be able to do the following operation to mitigate race
conditions as described here
(https://docs.djangoproject.com/en/2.1/ref/models/expressions/#avoiding-
race-conditions-using-f)
{{{
item = Item.objects.create()
# arbitrary number of other items created/destroyed etc
item_version_2 = ItemVersion.objects.create(
item=item,
version_number=Subquery(
item.versions.order_by('-version_number').annotate(
max_version_number=Coalesce(Max('version_number'), 0)
).annotate(
new_version_number=F('max_version_number) + 1
).values('new_version_number')
)
)
}}}
As written, I would expect the F() in the inner expression to always be
resolvable, because it is in a Subquery (and not the result of an insert).
However this query is blocked by the compiler because "F() expressions can
only be used to update, not to insert." Would it be possible to allow F
expressions in a Subquery even if it is being used in an insert? Is there
an edge case that I'm missing that caused the team to not consider this?
--
--
Ticket URL: <https://code.djangoproject.com/ticket/30129#comment:2>
Django
expressions
-------------------------------------+-------------------------------------
Reporter: Charlie McBride | Owner: nobody
Type: New feature | Status: new
Component: Database layer | Version: 2.1
(models, ORM) |
Severity: Normal | Resolution:
Expressions |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Changes (by Tim Graham):
* stage: Unreviewed => Accepted
Comment:
It seems okay, if it's feasible to implement.
--
Ticket URL: <https://code.djangoproject.com/ticket/30129#comment:3>
Django
expressions
-------------------------------------+-------------------------------------
Reporter: Charlie McBride | Owner:
Type: New feature | Status: assigned
Component: Database layer | Version: 2.1
(models, ORM) |
Severity: Normal | Resolution:
Keywords: Subquery, F, Query | Triage Stage: Accepted
Expressions |
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
* status: new => assigned
* owner: nobody => birthdaysgift
--
Ticket URL: <https://code.djangoproject.com/ticket/30129#comment:4>
Django
expressions
-------------------------------------+-------------------------------------
Type: New feature | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Keywords: Subquery, F, Query | Triage Stage: Accepted
Expressions |
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Changes (by birthdaysgift):
* owner: birthdaysgift => (none)
* status: assigned => new
--
Ticket URL: <https://code.djangoproject.com/ticket/30129#comment:5>
Django
expressions
-------------------------------------+-------------------------------------
Reporter: Charlie McBride | Owner: (none)
Type: New feature | Status: new
Component: Database layer | Version: 2.1
(models, ORM) |
Severity: Normal | Resolution:
Keywords: Subquery, F, Query | Triage Stage: Accepted
Expressions |
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Comment (by Sarah Boyce):
I think this is resolved (I'm not sure when), raised a PR just to show
that I think it's been fixed https://github.com/django/django/pull/16451
(but I will close the PR later).
I feel like I'm not supposed to close tickets so I'll let a DSF team
member check I haven't missed anything and close the ticket.
--
Ticket URL: <https://code.djangoproject.com/ticket/30129#comment:6>
Django
expressions
-------------------------------------+-------------------------------------
| Boyce
Type: New feature | Status: assigned
(models, ORM) |
Severity: Normal | Resolution:
Keywords: Subquery, F, Query | Triage Stage: Accepted
Expressions |
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
* owner: (none) => Sarah Boyce
* status: new => assigned
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/30129#comment:7>
Django
expressions
-------------------------------------+-------------------------------------
Reporter: Charlie McBride | Owner: Sarah
| Boyce
Type: New feature | Status: assigned
Component: Database layer | Version: 2.1
(models, ORM) |
Severity: Normal | Resolution:
Keywords: Subquery, F, Query | Triage Stage: Accepted
Expressions |
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Comment (by Tim Graham):
I did a bisect and confirmed this was fixed in Django 3.0 by
35431298226165986ad07e91f9d3aca721ff38ec.
--
Ticket URL: <https://code.djangoproject.com/ticket/30129#comment:8>
Django
expressions
-------------------------------------+-------------------------------------
Reporter: Charlie McBride | Owner: Sarah
| Boyce
Type: New feature | Status: assigned
Component: Database layer | Version: 2.1
(models, ORM) |
Severity: Normal | Resolution:
Keywords: Subquery, F, Query | Triage Stage: Accepted
Expressions |
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Comment (by Sarah Boyce):
Thank you Tim! Updated the commit message to credit 👍
--
Ticket URL: <https://code.djangoproject.com/ticket/30129#comment:9>
Django
expressions
-------------------------------------+-------------------------------------
Reporter: Charlie McBride | Owner: Sarah
| Boyce
Type: New feature | Status: assigned
Component: Database layer | Version: 2.1
(models, ORM) |
Severity: Normal | Resolution:
Keywords: Subquery, F, Query | Triage Stage: Accepted
Expressions |
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"05bcd5baafc0a5783923e32d2b7e2b7fff7d152a" 05bcd5ba]:
{{{
#!CommitTicketReference repository=""
revision="05bcd5baafc0a5783923e32d2b7e2b7fff7d152a"
Refs #30129 -- Added test for create() with F() expression in Subquery.
Fixed in 35431298226165986ad07e91f9d3aca721ff38ec.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30129#comment:10>
Django
expressions
-------------------------------------+-------------------------------------
Reporter: Charlie McBride | Owner: Sarah
| Boyce
Component: Database layer | Version: 2.1
(models, ORM) |
Keywords: Subquery, F, Query | Triage Stage: Accepted
Expressions |
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
--
Ticket URL: <https://code.djangoproject.com/ticket/30129#comment:11>