Changes between Version 1 and Version 2 of Ticket #32632, comment 4


Ignore:
Timestamp:
Apr 14, 2021, 9:50:59 PM (4 years ago)
Author:
Simon Charette

Legend:

Unmodified
Added
Removed
Modified
  • Ticket #32632, comment 4

    v1 v2  
    77After playing around with a few things here's a list of two proposed solutions
    88
    9 1. Remove the `tree.Node.add` branch for `if data in self.children`. While [https://djangoci.com/job/django-coverage/HTML_20Coverage_20Report/ it's covered by the suite] only [https://github.com/django/django/blob/ca9872905559026af82000e46cde6f7dedc897b6/tests/queries/tests.py#L1737-L1747 a single test in the whole suite reaches it] the branch under arguably convoluted conditions while the check is performed 99% of the time when `filter` and `exclude` are called to perform expensive comparisons of contained expressions. I wouldn't be surprised if it was at the origin of others recent slowdowns in the ORM query construction due to the introduction of `BaseExpression.identity` to needs for deconstruc-ability imposed by `Index(condition)` and `Constraint(condition)` for little benefit.
     91. Remove the `tree.Node.add` branch for `if data in self.children`. While [https://djangoci.com/job/django-coverage/HTML_20Coverage_20Report/ it's covered by the suite] only [https://github.com/django/django/blob/ca9872905559026af82000e46cde6f7dedc897b6/tests/queries/tests.py#L1737-L1747 a single test in the whole suite reaches] the branch under arguably convoluted conditions while the check is performed 99% of the time when `filter` and `exclude` are called to perform expensive O(n) operations. I wouldn't be surprised if it was at the origin of others recent slowdowns in the ORM query construction due to the introduction of `BaseExpression.identity` to needs for deconstruc-ability imposed by `Index(condition)` and `Constraint(condition)` for little benefit.
    10102. Make `Subquery` (and `Exists`) explicitly non-deconstructible; in practice they already are because `sql.Query` doesn't properly implement `deconstruct` so there's little value in implementing `identity` for them in the first place.
    11113. Remove the `sql.Query(BaseExpression)` subclassing as it's no longer necessary in practice. That'll have the side effect of removing support for `Model.objects.filter(foo=bar).query == Model.objects.filter(foo=bar).query` which is a nice property but since we don't internally have a need for it ''yet'' there's little point in the complexity it incurs.
Back to Top