Code

#19939 closed Bug (fixed)

Generic relations regression in split_exclude() cases.

Reported by: akaariai 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

Description

Generic relations do not work correctly in split_exclude situations in master. Compare 1.5 query of generic_relations/tests.py, 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.

Attachments (0)

Change History (2)

comment:1 Changed 14 months ago by akaariai

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Severity changed from Normal to Release blocker
  • Triage Stage changed from Unreviewed to Accepted

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

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

In c0d8932a6d03e9326a4e407e944b09bf43cf929c:

Fixed #19939 -- generic relations + split_exclude regression

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

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.