Opened 5 years ago

Closed 5 years ago

#19939 closed Bug (fixed)

Generic relations regression in split_exclude() cases.

Reported by: Anssi Kääriäinen Owned by: nobody
Component: Database layer (models, ORM) Version: master
Severity: Release blocker Keywords: generic relations
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no


Generic relations do not work correctly in split_exclude situations in master. Compare 1.5 query of generic_relations/, print(Animal.objects.exclude(tags__tag='fatty').query):

SELECT "generic_relations_animal"."id", "generic_relations_animal"."common_name", "generic_relations_animal"."latin_name"
FROM "generic_relations_animal"
WHERE NOT (("generic_relations_animal"."id" IN (
  SELECT U1."object_id" FROM "generic_relations_animal" U0
  INNER JOIN "generic_relations_taggeditem" U1 ON (U0."id" = U1."object_id")
  INNER JOIN "generic_relations_taggeditem" U2 ON (U0."id" = U2."object_id")
  INNER JOIN "generic_relations_taggeditem" U3 ON (U2."id" = U3."id")
  WHERE (U1."tag" = fatty  AND U3."content_type_id" = 14  AND U1."object_id" IS NOT NULL))
AND "generic_relations_animal"."id" IS NOT NULL))

to master's version:

SELECT "generic_relations_animal"."id", "generic_relations_animal"."common_name", "generic_relations_animal"."latin_name"
FROM "generic_relations_animal" WHERE NOT ("generic_relations_animal"."id" IN (
  SELECT U1."object_id" FROM "generic_relations_taggeditem" U1 WHERE U1."tag" = fatty ))

Now, the 1.5 version has a lot of completely needless extra joins and where conditions, but it has one important distinction to master's version: the U3."content_type_id" = 14 restriction. This is missing from master. The content type restriction is done by get_extra_join_sql() method, which can refer to both sides of the join. If the join is splitted, then this will not work.

I think the way to handle this is to add an extra_rel_restriction() method which gets the joined table's alias and returns a WhereNode instance. If done in this way the restriction can be applied in split_exclude() situations, too, and because of the use of WhereNode the return value isn't pure SQL which could cause problems for other backends. The wherenode can be applied directly into the join clause, or in the case of split_exclude() into the subquery's where condition. In addition the get_extra_join_sql() method was badly named in any case.

This wasn't spotted by tests, as the tests trying to spot this issue seem to rely on reset of primary key values between tests.

Change History (2)

comment:1 Changed 5 years ago by Anssi Kääriäinen

Severity: NormalRelease blocker
Triage Stage: UnreviewedAccepted

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

Resolution: fixed
Status: newclosed

In c0d8932a6d03e9326a4e407e944b09bf43cf929c:

Fixed #19939 -- generic relations + split_exclude regression

Added a test, the issue was already fixed (likely by the patch
for #19385).

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