#17000: sql/query.py add_q refactoring
-------------------------------------+-------------------------------------
Reporter: akaariai | Owner: nobody
Type: | Status: new
Cleanup/optimization | Version: 1.3
Component: Database layer | Resolution:
(models, ORM) | Triage Stage: Design
Severity: Normal | decision needed
Keywords: | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by akaariai):
I have a branch at github which does some refactoring to the
django/utils/tree.py and to add_q(). There are a couple of reasons why I
see some change is needed here:
1. add_q() is hard to understand
2. add_q() produces inefficient trees, which results in query.clone()
overhead, and as_sql() overhead
3. add_q() and add_filter(), and `WhereNode`.as_sql() only work if the
boolean tree is formed just like it is currently done in add_q(). When
using general boolean trees, the `WhereNode`.as_sql() falls down for
example (see pull request: https://github.com/django/django/pull/92)
4. the utils/tree.py contains some hard-to-use methods. More
specifically the .negate() method which isn't the same as changing
self.negated, and the start_subtree()/end_subtree() which prevents holding
a reference to a certain node of the tree. Do a_node.start_subtree(), then
the a_node is mutated in place and made empty and it will be a children of
a new node b_node, which essentially contains a_node
5. the refactoring fixes some bugs, see the added tests
Even the new version isn't as clean as I would like, but cleaning it up
more will lead to unmanageable amount of changes. I don't particularly
like the path argument from add_q() to add_filter() and would like to
invent a cleaner solution here. The solution seems to be to just maintain
one boolean tree up until execution stage, and split the tree into where
and having at that stage.
The work is available from
[https://github.com/akaariai/django/compare/refactor_utils_tree]. Note
that the idea is to make the logic easier to follow. If reviewing this, it
is important that the new logic must be easier to understand than the
current logic or there isn't much point in the work. It is hard for me to
see if the new logic is easier to understand to others.
--
Ticket URL: <https://code.djangoproject.com/ticket/17000#comment:4>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/django-updates?hl=en.
