Opened 2 years ago

Closed 2 years ago

#21376 closed Bug (fixed)

Regression 2 in query join promotion logic

Reported by: timo Owned by: akaariai
Component: Database layer (models, ORM) Version: master
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no


(master only) I think there's a regression in the following commit - test case that passes before the commit attached. I imagine we may want to introduce a new model rather than modify ObjectC as done in the patch.

(regression 1 was #21366)

Attachments (1)

21376_test.diff (1.4 KB) - added by timo 2 years ago.

Download all attachments as: .zip

Change History (6)

comment:1 Changed 2 years ago by akaariai

Is this really a regression? For the query to produce any results objectb must be 1, and if objectb is 1, then LEFT JOIN will not produce any more results. This of course assumes that foreign key constraints are in effect, but that assumption has always been there for the ORM.

Rewriting the test to show that wrong results are produced would make it clear what the regression is. Not producing a left join in a case where it was done earlier isn't a regression.

comment:2 Changed 2 years ago by timo

  • Owner changed from akaariai to timo
  • Status changed from new to assigned

I'll rewrite the test.

Changed 2 years ago by timo

comment:3 Changed 2 years ago by timo

  • Owner timo deleted
  • Status changed from assigned to new

Test should now be sufficient, I think.

comment:4 Changed 2 years ago by akaariai

  • Owner set to akaariai
  • Status changed from new to assigned
  • Triage Stage changed from Unreviewed to Accepted

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

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

In 6fe2b001dba45134d7c10729c57959995e241a88:

Fixed #21376 -- New implementation for query join promotion logic

This commit introduced a new class JoinPromoter that can be used to
abstract away join promotion problems for complex filter conditions.
Query._add_q() and Query.combine() now use the new class.

Also, added a lot of comments about why join promotion is done the way
it is.

Thanks to Tim Graham for original report and testing the changes, and
for Loic Bistuer for review.

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