Opened 2 years ago

Closed 2 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.

Attachments (0)

Change History (2)

comment:1 Changed 2 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 2 years ago by akaariai

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

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 2 years ago by akaariai (previous) (diff)

Add Comment

Modify Ticket

Change Properties
<Author field>
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'

E-mail address and user name can be saved in the Preferences.

Note: See TracTickets for help on using tickets.