Opened 4 years ago

Closed 4 years ago

#18741 closed Cleanup/optimization (fixed)

Small cleanup to split_exclude()

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

  • Triage Stage changed from Accepted to Ready 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 4 years ago by akaariai

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

In [c1684e3dcb2adf0fec8fd423cc73122330c268fe]:

(The changeset message doesn't reference this ticket)

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

Version 0, edited 4 years ago by akaariai (next)
Note: See TracTickets for help on using tickets.
Back to Top