Code

Opened 2 years ago

Closed 2 years ago

#18014 closed Cleanup/optimization (fixed)

Remove rev_join_map from sql/query.py

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

Description

The sql.query.QuerySet.rev_join_map is used currently in two places of the query.py:

  1. In query.combine(). The information used there is available directly from alias_map
  2. In relabel_aliases as a fast path to join_map. However, one can just iterate through the join_map and relabel the aliases directly.

It is hard to see if the usage in 1. is actually currently correct or not: when combining queries the joins are re-added using _table_ name for the left hand side of the join (the thing we are joining into). I am pretty sure the correct way would be to do that using the existing lhs _alias_ so that the joins are added to the same previous join as in the original query.

While the cleanup doesn't promise much advancement, it would give a (really) small speedup to query cloning. Just getting rid of one variable is reason enough for removal I guess.

Attachments (1)

18014.diff (3.2 KB) - added by akaariai 2 years ago.

Download all attachments as: .zip

Change History (5)

comment:1 Changed 2 years ago by akaariai

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

The attached patch does the mentioned removal. I am actually pretty sure the rev_join_map usage in query.combine is broken: the intention is to use the left hand side table's alias, but rev_join_map has the table instead of the alias. All tests passed on SQLite3.

Changed 2 years ago by akaariai

comment:2 Changed 2 years ago by claudep

  • Has patch set
  • Triage Stage changed from Unreviewed to Accepted
  • Type changed from Uncategorized to Cleanup/optimization

comment:3 Changed 2 years ago by Alex

Looks pretty reasonable to me, check in at your discretion!

comment:4 Changed 2 years ago by akaariai

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

In [17878]:

Fixed #18014 -- Removed rev_join_map from sql/query.py.

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.