Code

Opened 5 months ago

Closed 3 months ago

Last modified 3 months ago

#21413 closed Bug (fixed)

select_related "row, fields misalignment" in SQLCompiler.fill_related_selections()

Reported by: anonymous Owned by: nobody
Component: Database layer (models, ORM) Version: master
Severity: Normal Keywords: select_related parent inheritance subclass oracle
Cc: shai Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by shai)

Given this models for example:

class Place(models.Model):
    name = models.CharField(max_length=50)
    address = models.CharField(max_length=80)

class Restaurant(Place):
    serves_hot_dogs = models.BooleanField()
    serves_pizza = models.BooleanField()

When using select related to prefetch the child, the child fields are not initialized properly:

>>> Restaurant.objects.create(name='Bobs Cafe', address="Somewhere", serves_pizza=True, serves_hot_dogs=True)
<Restaurant: Restaurant object>

>>> p = Place.objects.all().select_related('restaurant')[0]

>>> vars(p.restaurant)
{'_place_ptr_cache': <Place: Place object>,
 '_state': <django.db.models.base.ModelState at 0x2e791d0>,
 'address': u'Somewhere',
 'id': 1L,
 'name': u'Bobs Cafe',
 'place_ptr_id': 1L,
 'serves_hot_dogs': 1,
 'serves_pizza': 1}

serves_pizza and serves_hot_dogs are set to 1 instead of True!
The cause of this bug is kind of tricky. It is actually a general defect because of a "row, fields misalignment" in SQLCompiler.fill_related_selections().
https://github.com/django/django/blob/master/django/db/models/sql/compiler.py#L650:
self.query.related_select_cols returns:

[SelectInfo(col=(u't13781_restaurant', u'place_ptr_id'), field=<django.db.models.fields.AutoField: id>), 
SelectInfo(col=(u't13781_restaurant', 'serves_hot_dogs'), field=<django.db.models.fields.CharField: name>), 
SelectInfo(col=(u't13781_restaurant', 'serves_pizza'), field=<django.db.models.fields.CharField: address>)]

But it only turns up when using SQL backends that use the misaligned fields for converting the row values depending on the field type (mainly when resolve_columns() is implemented). So for example in MySQL this only happens when boolean fields on the child model are involved, because in this special case the field information is interpreted.
https://github.com/django/django/blob/master/django/db/backends/mysql/compiler.py#L10:

if (field and field.get_internal_type() in ("BooleanField", "NullBooleanField") and
  value in (0, 1)):
  value = bool(value)

It won't appear using SQLite but i guess it will appear using Oracle (only tested SQLite and MySQL).

It might be somehow related to #21203 and #21126.

Attachments (1)

21413.diff (3.1 KB) - added by slide21@… 5 months ago.
patch for ticket #21413

Download all attachments as: .zip

Change History (6)

Changed 5 months ago by slide21@…

patch for ticket #21413

comment:1 Changed 5 months ago by shai

  • Cc shai added
  • Description modified (diff)
  • Keywords oracle added
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

I haven't reviewed the patch, just gave the bug a quick look. Replaced explicit links to tickets in the description with ticket references.

comment:2 Changed 5 months ago by akaariai

  • Triage Stage changed from Unreviewed to Accepted

Proposed patch for master here: https://github.com/django/django/pull/1929

I am not sure if the as_pairs change is too much to backpatch. Of course, this is a change to private API and get_default_columns(as_pairs=True) isn't likely that used.

comment:3 Changed 5 months ago by shai

I just tested both PR 1929 and the patch attached to the ticket against Oracle.

Both include the test that fails on master without the fix, the pull request does it more elegantly (where the models for the test already exist).

PR 1929's fix includes an "assert" statement which fails when the test is executed. The patch passes (and its fix appears much simpler). Anssi, do you care to explain what is wrong with it?

comment:4 Changed 3 months ago by Anssi Kääriäinen <akaariai@…>

  • Resolution set to fixed
  • Status changed from new to closed

In 9918c11114ac3ec9622631558ef26ebf3919cb69:

Fixed #21413 -- resolve_columns fields misalignment

comment:5 Changed 3 months ago by Anssi Kääriäinen <akaariai@…>

In 0f272629ca18e440aef67b4a3fd9377a57fb25a8:

[1.6.x] Fixed #21413 -- resolve_columns fields misalignment

Backpatch of 9918c11114ac3ec9622631558ef26ebf3919cb69 from master.

Conflicts:

django/db/models/sql/compiler.py
tests/model_inheritance_regress/tests.py

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.