Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#31246 closed Bug (fixed)

select_for_update() with "of" uses wrong tables from (multi-level) model inheritance.

Reported by: Abhijeet Viswa Owned by: Abhijeet Viswa
Component: Database layer (models, ORM) Version: 2.2
Severity: Release blocker Keywords: select_for_update mti relations
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Abhijeet Viswa)

Two different bugs related to model inheritance:
1.
Consider the following models:

class A(models.Model):
    field1 = models.IntegerField()

class B(A):
    field2 = models.IntegerField()

class C(models.Model):
    b = models.ForeignKey(B, on_delete=models.CASCADE)
queryset = C.objects.select_related('b').select_for_update(of=('b'))

The above queryet generates the following query:

SELECT "selectforupdate_c"."id",
       "selectforupdate_c"."b_id",
       "selectforupdate_a"."id",
       "selectforupdate_a"."field1",
       "selectforupdate_b"."a_ptr_id",
       "selectforupdate_b"."field2"
FROM   "selectforupdate_c"
       INNER JOIN "selectforupdate_b"
               ON ( "selectforupdate_c"."b_id" =
                  "selectforupdate_b"."a_ptr_id" )
       INNER JOIN "selectforupdate_a"
               ON ( "selectforupdate_b"."a_ptr_id" = "selectforupdate_a"."id" )
FOR UPDATE OF "selectforupdate_a"

The of clause in the generated SQL references the model a instead of b

2.
Consider the following models:

class A(models.Model):
    field1 = models.IntegerField()

class B(A):
     field2 = models.IntegerField()
 
class C(B):
    field_c = models.IntegerField()
queryset = C.objects.all().select_for_update(of=('b_ptr', 'b_ptr__a_ptr'))


The above queryset crashes with the following crashlog:

Traceback (most recent call last):
  File "/home/abhijeet/git-repos/django/django/core/management/base.py", line 328, in run_from_argv
    self.execute(*args, **cmd_options)
  File "/home/abhijeet/git-repos/django/django/core/management/base.py", line 369, in execute
   output = self.handle(*args, **options)
  File "/home/abhijeet/django_contrib/selectforupdate/management/commands/debug.py", line 12, in handle
    print(queryset.query)
  File "/home/abhijeet/git-repos/django/django/db/models/sql/query.py", line 257, in __str__
    sql, params = self.sql_with_params()
  File "/home/abhijeet/git-repos/django/django/db/models/sql/query.py", line 265, in sql_with_params
    return self.get_compiler(DEFAULT_DB_ALIAS).as_sql()
  File "/home/abhijeet/git-repos/django/django/db/models/sql/compiler.py", line 555, in as_sql
    of=self.get_select_for_update_of_arguments(),
  File "/home/abhijeet/git-repos/django/django/db/models/sql/compiler.py", line 1036, in get_select_for_update_of_arguments
    select_index = klass_info['select_fields'][0]
IndexError: list index out of range

This is related to #30953

Change History (11)

comment:1 by Abhijeet Viswa, 5 years ago

Description: modified (diff)

comment:2 by Simon Charette, 5 years ago

Keywords: select_for_update; mti; relations; → select_for_update mti relations
Severity: NormalRelease blocker
Triage Stage: UnreviewedAccepted

comment:3 by Mariusz Felisiak, 5 years ago

Version: master2.2

comment:4 by Abhijeet Viswa, 5 years ago

Has patch: set

I forgot to link to my PR:

PR

comment:5 by Mariusz Felisiak, 5 years ago

Needs tests: set
Patch needs improvement: set
Summary: Related Model with inheritance generates wrong select_for_update 'of'select_for_update() with "of" uses wrong tables from (multi-level) model inheritance.

It is a data loss issue introduced in 0107e3d1058f653f66032f7fd3a0bd61e96bf782 (Django 2.2.8).

comment:6 by Abhijeet Viswa, 5 years ago

Description: modified (diff)

comment:7 by Abhijeet Viswa, 5 years ago

Description: modified (diff)

comment:8 by Mariusz Felisiak, 5 years ago

Needs tests: unset
Patch needs improvement: unset
Triage Stage: AcceptedReady for checkin

comment:9 by Mariusz Felisiak <felisiak.mariusz@…>, 5 years ago

Resolution: fixed
Status: assignedclosed

In 1712a76:

Fixed #31246 -- Fixed locking models in QuerySet.select_for_update(of=()) for related fields and parent link fields with multi-table inheritance.

Partly regression in 0107e3d1058f653f66032f7fd3a0bd61e96bf782.

comment:10 by Mariusz Felisiak <felisiak.mariusz@…>, 5 years ago

In 8faaaf4e:

[3.0.x] Fixed #31246 -- Fixed locking models in QuerySet.select_for_update(of=()) for related fields and parent link fields with multi-table inheritance.

Partly regression in 0107e3d1058f653f66032f7fd3a0bd61e96bf782.

Backport of 1712a76b9dfda1ef220395e62ea87079da8c9f6c from master

comment:11 by Mariusz Felisiak <felisiak.mariusz@…>, 5 years ago

In 32d89bf:

[2.2.x] Fixed #31246 -- Fixed locking models in QuerySet.select_for_update(of=()) for related fields and parent link fields with multi-table inheritance.

Partly regression in 0107e3d1058f653f66032f7fd3a0bd61e96bf782.

Backport of 1712a76b9dfda1ef220395e62ea87079da8c9f6c from master.

Note: See TracTickets for help on using tickets.
Back to Top