Opened 5 years ago

Closed 5 years ago

#18741 closed Cleanup/optimization (fixed)

Small cleanup to split_exclude()

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


sql.query.split_exclude() contains this call:

query = Query(self.model)        
query.add_filter(filter_expr, can_reuse=can_reuse)

Here a new subquery is created, and the can_reuse set is a set of joins that are reusable from the outer query.

The problem is that passing the can_reuse here doesn't make any sense - the new query contains no joins so there of course isn't anything to reuse. If the .add_filter() adds something to the can_reuse set, that addition will be incorrect for the outer query. However I am not sure if it is possible to create an actual error because of this.

This also makes it slightly easier to see what happens in split_exclude().

When I got rid of can_reuse all tests pass.

Change History (2)

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

Triage Stage: AcceptedReady for checkin

A patch in

Marking this RFC as I am pretty sure the patch is correct. Hopefully I will get this (and some other pending patches) committed during the weekend.

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

Resolution: fixed
Status: newclosed

In [c1684e3dcb2adf0fec8fd423cc73122330c268fe]:

 Fixed #18731 -- Cleaned up split_exclude's use of can_reuse

 The outer query's set of reusable joins (can_reuse) was passed to the
 inner query's add_filter call. This was incorrect.

Note that the ticket number is wrong in the commit message.

Last edited 5 years ago by Anssi Kääriäinen (previous) (diff)
Note: See TracTickets for help on using tickets.
Back to Top