[Django] #32888: Using select_for_update with self as an of-argument gets dropped when only selecting fields on related model
Django
selecting fields on related model
-------------------------------------+-------------------------------------
Reporter: Hannes | Owner: nobody
Ljungberg |
Type: Bug | Status: new
Component: Database | Version: dev
layer (models, ORM) |
Severity: Normal | Keywords:
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
The existing test "test_for_update_of_self_when_self_is_not_selected" can
be extended to demonstrate this behaviour:
{{{
diff --git a/tests/select_for_update/tests.py
b/tests/select_for_update/tests.py
index 705975b760..00fa102d33 100644
--- a/tests/select_for_update/tests.py
+++ b/tests/select_for_update/tests.py
@@ -240,9 +240,15 @@ class SelectForUpdateTests(TransactionTestCase):
select_for_update(of=['self']) when the only columns selected are
from
related tables.
"""
- with transaction.atomic():
+ with transaction.atomic(), CaptureQueriesContext(connection) as
ctx:
values =
list(Person.objects.select_related('born').select_for_update(of=('self',)).values('born__name'))
self.assertEqual(values, [{'born__name': self.city1.name}])
+ if connection.features.select_for_update_of_column:
+ expected = ['select_for_update_person"."id']
+ else:
+ expected = ['select_for_update_person']
+ expected = [connection.ops.quote_name(value) for value in
expected]
+ self.assertTrue(self.has_for_update_sql(ctx.captured_queries,
of=expected))
@skipUnlessDBFeature('has_select_for_update_nowait')
def test_nowait_raises_error_on_block(self):
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32888>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
selecting fields on related model
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Hannes Ljungberg):
Tried to understand what's happening in
`SQLCompiler.get_select_for_update_of_arguments` and it appears like it's
dependant on what tables and columns have been SELECT:ed. Looks like we
only need to do this because Oracle want columns instead of tables. I
wonder if we could base this on the JOIN:ed tables instead and grab the
column from the predicate for Oracle?
--
Ticket URL: <https://code.djangoproject.com/ticket/32888#comment:1>
Django
selecting fields on related model
-------------------------------------+-------------------------------------
Component: Database layer | Version: dev
(models, ORM) |
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => closed
* resolution: => invalid
Comment:
IMO this is an expected behavior, we changed it in the
[https://github.com/django/django/pull/12154 PR].
--
Ticket URL: <https://code.djangoproject.com/ticket/32888#comment:2>
Django
selecting fields on related model
-------------------------------------+-------------------------------------
Type: Bug | Status: closed
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution: invalid
Keywords: | Triage Stage:
| Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Hannes Ljungberg):
Thanks, missed that PR. IMO it's a bit unexpected that the selected fields
might end up affecting `FOR UPDATE`. What do you think about adding a
small note about this in the docs?
--
Ticket URL: <https://code.djangoproject.com/ticket/32888#comment:3>
Django
selecting fields on related model
-------------------------------------+-------------------------------------
Type: Bug | Status: closed
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution: invalid
Keywords: | Triage Stage:
| Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak):
Replying to [comment:3 Hannes Ljungberg]:
> Thanks, missed that PR. IMO it's a bit unexpected that the selected
fields might end up affecting `FOR UPDATE`. What do you think about adding
a small note about this in the docs?
Sure, feel-free to propose a clarification.
--
Ticket URL: <https://code.djangoproject.com/ticket/32888#comment:4>
Django
selecting fields on related model
-------------------------------------+-------------------------------------
Type: | Status: new
Cleanup/optimization |
Component: Documentation | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: closed => new
* resolution: invalid =>
* has_patch: 0 => 1
* component: Database layer (models, ORM) => Documentation
* type: Bug => Cleanup/optimization
Comment:
[https://github.com/django/django/pull/14891 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/32888#comment:5>
Django
are not selected.
-------------------------------------+-------------------------------------
Reporter: Hannes Ljungberg | Owner: Hannes
Type: | Ljungberg
Cleanup/optimization | Status: assigned
Component: Documentation | Version: dev
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: nobody => Hannes Ljungberg
* status: new => assigned
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/32888#comment:6>
Django
are not selected.
-------------------------------------+-------------------------------------
Reporter: Hannes Ljungberg | Owner: Hannes
Type: | Ljungberg
Component: Documentation | Version: dev
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"d400b08a8baa697905daadd47a6ba12336e93336" d400b08a]:
{{{
#!CommitTicketReference repository=""
revision="d400b08a8baa697905daadd47a6ba12336e93336"
Fixed #32888 -- Doc'd that select_for_update() only locks tables with
selected columns.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32888#comment:7>
Django
are not selected.
-------------------------------------+-------------------------------------
Reporter: Hannes Ljungberg | Owner: Hannes
Type: | Ljungberg
Cleanup/optimization | Status: closed
Component: Documentation | Version: dev
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"816e80930251497635b97a772cc832edf09cdede" 816e8093]:
{{{
#!CommitTicketReference repository=""
revision="816e80930251497635b97a772cc832edf09cdede"
[4.0.x] Fixed #32888 -- Doc'd that select_for_update() only locks tables
with selected columns.
Backport of d400b08a8baa697905daadd47a6ba12336e93336 from main
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32888#comment:8>