#10790: Too many joins in a comparison for NULL.
-------------------------------------+-------------------------------------
Reporter: mtredinnick | Owner: akaariai
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Ready for
Keywords: | checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by akaariai):
I removed the join_cache as I haven't been able to show it improves
performance in any of the djangobench benchmarks. If needed it is fairly
simple to re-add the cache.
What the patch does is that it splits setup_joins() into two parts.
Currently we do "for each lookup in the given lookup path, resolve lookup
part, immediately add the found joins to query". This is changed to "First
resolve the whole lookup path. After resolve add the joins into the
query".
The names_to_path() method does the resolving. It is somewhat complex, but
the code is pretty much exactly what we had before in setup_joins() minus
the actual join generation. As a result of the split the actual join setup
is around 10 lines and trim_joins() is made cleaner, too.
While for this ticket this refactoring isn't the minimal change, the work
done allows doing two further improvements:
1. Moving of the names_to_path() call into the beginning of add_filter()
- this in turn will allow resolving some more bugs (for example F() are
currently double-added in multi_join exclude cases), and will also make
custom lookups much easier to add (see #16187).
2. Moving the names_to_path() method to model._meta. This will allow
custom meta objects to alter the behaviour of join generation, and will
also allow code outside sql/query.py to reuse the same resolution code
(Admin for example has duplicate code for this).
So, the patch isn't about this ticket alone.
Reading the patch itself is nearly impossible. The diff generated just
happens to interleave badly. After applying the patch the resulting code
should be more readable than the old code. Is there some specific part of
the resulting code which seems hard to understand? I would really
appreciate if you could do a side-by-side read of the old and new
setup_joins() + trim_joins(). If the old code is easier to understand than
the new code then that is a problem. This is of course a possibility - it
is easy for me to understand the code I just wrote...
This change is a big change, and big changes to ORM tend to be scary. In
my opinion the change is needed. Now seems to be the perfect time to add
this into master. 1.6 is more than six months away so we have plenty of
time to find regressions and fix them.
--
Ticket URL: <https://code.djangoproject.com/ticket/10790#comment:46>
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 [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit https://groups.google.com/groups/opt_out.
