On Saturday, December 21, 2013 3:15:12 AM UTC+2, Josh Smeaton wrote: > > I'm interested in tackling > ticket#14030<https://code.djangoproject.com/ticket/14030>, > ideally before the Jan 20th cutoff, but I'm looking for some guidance. > There have been a lot of references to Anssi Kääriäinen attempting an ORM > refactor before trying to fix aggregates, but I'm not sure if this is still > a goal or not. >
Yes, more refactoring is still the plan. A lot has already been done. The main part of the work has been bug fixing, code cleanup and join generation changes. The next part is introducing custom lookups for 1.7 (see https://github.com/django/django/pull/2019), and later on in 1.8 allow usage of custom lookups in every part of the ORM. This means one can do queries such as .order_by(' jsonfield__user__firstname'), or more relevant to this ticket, .annotate(avg_year=Avg('datefield__year')). To make the above possible Django's ORM needs to switch to using unified query expression API. For example aggregates, custom lookups and Expression nodes will all respond to the same API. The API deals mainly with how to produce SQL for the expressions, how to convert db values to Python values and things like that. As a side effect Avg(F('somefield') + F('anotherfield')) should just work. > The work done by nateb at https://github.com/django/django/pull/46 was > very similar to the structure I believed would be the right way forward - > refactoring Aggregates to inherit from ExpressionNode. Unfortunately, the > patch submitted has been closed because it no longer applies cleanly. There > was also discussion regarding type coercion and a lack of real world tests > that should be considered. > > What I'd like to do is attempt to rewrite the above PR so that it applies > cleanly. The reviews that had already happened on that patch would then > mostly apply to the rewrite, eliminating the need for a start to finish > review of the concept in general. Is this something that I should go > through with, or are we better off waiting for the ORM refactor then > reevaluating the way forward? > Making Aggregates inherit from ExpressionNode shouldn't make the unified query expression API significantly easier or harder to achieve. If the code complexity stays manageable, I don't see a reason to not merge the changes. The change will be needed for Avg('somefield') + Avg('otherfield') support even when unified query expression API was implemented. One of the main problems for this ticket has been the code complexity. Both Aggregates and Expressions are somewhat complicated in the ORM. The combination of the two results in even worse code complexity. On the other hand this feature is clearly needed by many users, so maybe some code complexity can be tolerated. - Anssi -- You received this message because you are subscribed to the Google Groups "Django developers" group. To unsubscribe from this group and stop receiving emails from it, send an email to django-developers+unsubscr...@googlegroups.com. To post to this group, send email to django-developers@googlegroups.com. Visit this group at http://groups.google.com/group/django-developers. To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/1272168b-84f1-4968-a604-84141c174483%40googlegroups.com. For more options, visit https://groups.google.com/groups/opt_out.
