Code

Opened 10 months ago

Closed 9 months ago

Last modified 9 months ago

#21203 closed Bug (fixed)

Potential data corruption issue with Oracle and Mysql due to SQLCompiler.resolve_columns row, fields misalignment when using defer and select_related

Reported by: manfre Owned by: nobody
Component: Database layer (models, ORM) Version: 1.5
Severity: Release blocker Keywords:
Cc: akaariai Triage Stage: Ready for checkin
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

SQLCompiler.resolve_columns fields, values are misaligned with defer and select_related. Same data corruption issue as reported/fixed by issue #21126.

Given:

class Parent(models.Model):
	parentid = models.AutoField(primary_key=True)
	parent_bool = models.BooleanField(default=True)
	created = models.DateTimeField(auto_now=True)

class Child(models.Model):
	childid = models.AutoField(primary_key=True)
	parent = models.ForeignKey(Parent)

The following query

p = Parent.objects.create(parent_bool=True)
c = Child.objects.create(parent=p)

qs = Child.objects.all().select_related('parent').defer('parent__parent_bool')

will result in this field misalignment

        field=<django.db.models.fields.AutoField: childid>      value=1
        field=<django.db.models.fields.related.ForeignKey: parent>      value=1
        field=<django.db.models.fields.AutoField: parentid>     value=1
        field=<django.db.models.fields.BooleanField: parent_bool>       value=datetime.datetime(2013, 9, 30, 19, 59, 41, 490000, tzinfo=<UTC>)
        field=<django.db.models.fields.DateTimeField: created>  value=None

parent_bool should have been removed from fields, but it was not because results_iter doesn't properly compare the only_load fields to the correct db_table1.

1 https://github.com/django/django/blob/master/django/db/models/sql/compiler.py#L716

Attachments (0)

Change History (5)

comment:1 Changed 9 months ago by akaariai

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

Pull request https://github.com/django/django/pull/1698 should solve this ticket.

The basic problem was that the only_load dictionary wasn't used correctly.

Unfortunately the only_load dictionary also contains wrong data in some complex situations, but that is not this ticket's problem. I will create another ticket for that issue.

comment:2 Changed 9 months ago by akaariai

  • Triage Stage changed from Unreviewed to Accepted

comment:3 Changed 9 months ago by manfre

  • Triage Stage changed from Accepted to Ready for checkin

The pull request fixes the simple example I included above and the issue as experienced in my full project.

Here's a pull request for the backpatch to 1.5.x :)

https://github.com/django/django/pull/1701

(ignore previous edit, venv was corrupted and including an unpatched django from a different venv)

Last edited 9 months ago by manfre (previous) (diff)

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

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

In 9972a101e60d1c4bf1e0b6f3c7946c8716022711:

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

In queries using .defer() together with .select_related() the values
and fields arguments didn't align properly for resolve_columns().

Backpatch of bf13c75c0d94d606b8a077ff73bbd0440f05b396 from master.

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

In 7ebd10019d5c6c173825a41c5d435c80bee7b82a:

[1.5.x] Fixed #21203 -- resolve_columns fields misalignment

In queries using .defer() together with .select_related() the values
and fields arguments didn't align properly for resolve_columns().

Backpatch of 8c27247397cf16b17d0153ae059593c5a468de01 from master.

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.