[Django] #32944: Simplify where clause creation where the WhereNode is immediately created and then added to and then returned.
Django
and then added to and then returned.
-------------------------------------+-------------------------------------
Reporter: Keryn | Owner: nobody
Knight |
Type: | Status: assigned
Cleanup/optimization |
Component: Database | Version: dev
layer (models, ORM) |
Severity: Normal | Keywords:
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
There are currently 2 places in Django which essentially do:
{{{
where = WhereNode()
where.add(condition, AND)
}}}
and then return the where.
There are a ''bunch'' of places where a where class is added to and then
conditionally added to either immediately or far later in other complex
branches, (eg: is it a reffed expression, etc). Those are not for
consideration IMHO because the changes would probably add more complexity.
But for those 2 places, `django.db.models.sql.query.Query.build_filter`
and
`django.contrib.contenttypes.fields.GenericRelation.get_extra_restriction`
where the code is respectively:
{{{
clause = self.where_class()
clause.add(condition, AND)
return clause, []
}}}
and
{{{
cond = where_class()
lookup = field.get_lookup('exact')(field.get_col(remote_alias),
contenttype_pk)
cond.add(lookup, 'AND')
return cond
}}}
both can be simplified to respectively:
{{{
return self.where_class([condition], connector=AND), []
}}}
and
{{{
return where_class([lookup], connector=AND)
}}}
which end up as the same result, that is:
{{{
x = WhereNode([('a', 1)], connector='AND')
y = WhereNode()
y.add(('a', 1), 'AND')
assert x == y
}}}
but a bit faster. Specifically the proposed form is roughly `550-600ns`
per op vs. the unnecessary add which is `650-700ns`:
{{{
In [17]: %%timeit
...: x = WhereNode([('a', 1)], connector='AND')
597 ns ± 12.5 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)
In [18]: %%timeit
...: x = WhereNode()
...: x.add(('a', 1), 'AND')
780 ns ± 8.73 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)
}}}
Given `build_filter` prevalence (it's called what, at least once per
argument to `filter()` or `exclude()` ... maybe more on complex queries?)
it's maybe worth having.
I'll put in a PR shortly to run the bits of the test suite I can't, and
boil the ocean on the off-chance that this gets accepted. Naturally if
anything fails, we can just move it to invalid without further
consideration :)
--
Ticket URL: <https://code.djangoproject.com/ticket/32944>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
and then added to and then returned.
-------------------------------------+-------------------------------------
Type: | Knight
Cleanup/optimization | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: nobody => Keryn Knight
--
Ticket URL: <https://code.djangoproject.com/ticket/32944#comment:1>
Django
and then added to and then returned.
-------------------------------------+-------------------------------------
Type: | Knight
Cleanup/optimization | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
* has_patch: 0 => 1
* stage: Unreviewed => Accepted
Comment:
[https://github.com/django/django/pull/14665 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/32944#comment:2>
Django
and then added to and then returned.
-------------------------------------+-------------------------------------
Type: | Knight
Component: Database layer | Version: dev
(models, ORM) |
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"6a970a8b4600eb91be25f38caed0a52269d6303d" 6a970a8]:
{{{
#!CommitTicketReference repository=""
revision="6a970a8b4600eb91be25f38caed0a52269d6303d"
Fixed #32944 -- Avoided unnecessary WhereNode.add() calls.
Co-authored-by: Mariusz Felisiak <felisiak...@gmail.com>
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32944#comment:3>