Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#19672 closed Bug (fixed)

Negated Q objects over nullable joins result in invalid SQL

Reported by: ikelly Owned by: nobody
Component: Database layer (models, ORM) Version: master
Severity: Normal Keywords: Q ForeignKey nullable
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

Using the following set of models:

from django.db import models

class A(models.Model):
    value = models.IntegerField(null=True)

class B(models.Model):
    a = models.ForeignKey(A)

class C(models.Model):
    b = models.ForeignKey(B, null=True)

The query C.objects.filter(~Q(b__a__value=42)) fails with the following error:

DatabaseError: no such column: testapp_b.value

The SQL generated is:

SELECT "testapp_c"."id", "testapp_c"."b_id" FROM "testapp_c" LEFT OUTER JOIN "t
estapp_b" ON ("testapp_c"."b_id" = "testapp_b"."id") LEFT OUTER JOIN "testapp_a"
 ON ("testapp_b"."a_id" = "testapp_a"."id") WHERE NOT (("testapp_a"."value" = 42
  AND NOT ("testapp_b"."id" IS NULL) AND "testapp_b"."value" IS NOT NULL))

The problem is caused by the last WHERE condition, which is incorrectly filtering on the "value" column in the testapp_b table (where it does not exist) instead of in the testapp_a table.

Attachments (1)

19672.diff (3.1 KB) - added by ikelly 3 years ago.

Download all attachments as: .zip

Change History (6)

Changed 3 years ago by ikelly

comment:1 Changed 3 years ago by ikelly

  • Has patch set
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

The bug seems to happen because a local variable gets clobbered in the Query.add_filter method. I've uploaded a patch and tests to fix this.

comment:2 Changed 3 years ago by akaariai

  • Triage Stage changed from Unreviewed to Ready for checkin

The patch looks good to me.

What versions to patch? I think 1.4 is out of reach, this isn't a crashing or data loss bug.

1.5 is technically also in "crash, data-loss, security, regression" fixes only as it is in RC, but as we are going to do RC2 and the patch is as safe as they get, IMO master + stable/1.5.x seems like the correct way.

comment:3 Changed 3 years ago by ikelly

That sounds reasonable to me.

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

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

In e8fc3f3783744e4fcd3ff0ac43e3ce2572292f3a:

Added a test for negated Q object querying

The added test is from the patch in ticket #19672 (written by Ian
Kelly). Fixed #19672, refs #19849.

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

In 8ad436636fd385abd144274952b0c066885af042:

[1.5.x] Fixed #19672 -- Error in negated Q() filtering

There was a variable overwrite error in negated join filtering. This
happened when add_filter() was adding the IS NULL condition to the
WHERE clause.

This is not a backport from master as there have been some other
refactorings which made this patch irrelevant.

The patch is from Ian Kelly.

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