This thread caught my interest (because of potential implications for 
django-hvad).

While the proposed changes break django-hvad, I've already written a simple 
patch for django-hvad, so this is a non-issue. (The problem was the cleanup 
of Query.where. master and 1.4 had lots of calls to start_subtree which 
added WhereNodes into Query.where.children, those children are now tuples 
which broke our code. However the tuples make a lot more sense).

Regarding the actual patch, I like the cleanup in add_q, it's a lot nicer 
to read now (although needs_having is a bit crazy).

On Saturday, August 25, 2012 9:35:02 PM UTC+2, Anssi Kääriäinen wrote:
>
> I have done some more ORM refactoring work. I thought it would be a 
> good idea to post a summary of what is going on. 
>
> First, I haven't committed the utils.tree refactoring patch I was 
> planning to commit [https://github.com/akaariai/django/commits/ 
> refactor_utils_tree]. The reason is that I now feel that add_filter() 
> and deeper levels of the Query class need some refactorings first. 
>
> There is some work going on in the refactoring of the deeper levels of 
> the ORM. I committed join promotion improvement patch today: [https:// 
> code.djangoproject.com/ticket/16715#comment:25]. I have more planned 
> improvements to commit: 
>
>
> * Remove "dupe avoidance" logic from the ORM (#18748). 
> This is removing some code I strongly believe isn't needed any more. 
> This is still pending Malcolm's review. 
>
> * Fix join promotion in disjunctive filters (#18854). 
> This one should make join promotion in disjunction cases cleaner and 
> more efficient (as in less OUTER JOINS). In addition, this will make 
> add_q/add_filter a bit easier to understand. The problem currently is 
> that the first ORed filter is added to the query with connector=AND, 
> the rest of the filters in the disjunction with connector=OR. This is 
> done for join promotion reasons. However, this is very confusing when 
> trying to work with add_q and add_filter, and doesn't actually work 
> that well (see the added tests in the ticket's patch). 
>
> * Remove "trim" argument from add_filter (#18816). 
> The trim argument is only needed for split_exclude (that is, pushing 
> negated m2m filters to subqueries). So, add_filter (and then also 
> trim_joins) needs to handle split_exclude's special case. Handling 
> this case inside split_exclude() turned out to be a little ugly, but 
> IMO less ugly than the trim flag. This also allows further cleanups in 
> the following item. 
>
> * A biggie: complete refactoring of setup_joins (#10790, a separate 
> ticket could be good for this). 
> Best described in the last post in the ticket [https:// 
> code.djangoproject.com/ticket/10790#comment:41]. The patch isn't 
> completely ready (there are still stale comments floating around, the 
> commit history is ugly). 
>
>
> Applying the above patches should make the ORM code easier to follow. 
> Further features like custom lookups should be easier to add. The 
> produced queries should be of higher quality (less joins, less left 
> joins). And, it should be easier to do cleanups in the ORM. 
>
> A note about extra_filters: The setup_joins() refactoring removes the 
> "extra_filters" system used for generic relations. The system adds new 
> filters to the query for any joins generated by generic relations. 
> However, these are pushed into the WHERE clause, and this doesn't work 
> nicely with LOUTER JOINS. The refactoring adds the extra condition 
> directly into the join's ON clause, thus producing correct left outer 
> join conditions. 
>
> The extra_filters is (as far as I know) private API, but I'd like to 
> know if this is heavily used by the community. If so, it is easy 
> enough to leave this API in place (while still fixing the generic 
> relations stuff). 
>
> I hope I can get reviews for the above tickets. Getting reviews from 
> people who do not know the ORM is valuable, too, as one of the goals 
> is to make the ORM easier to understand. As the author I can't easily 
> see if my attempts to make the code easier to follow actually improve 
> anything. 
>
> Even if I do not get any reviews, I think it is a good idea to push 
> these patches in. Thus far it has been hard to get reviews for larger 
> ORM patches, and I am afraid that the refactoring work will stall if I 
> have to wait for a full review for every patch. If you want to review 
> the patches, but don't have time just now, please make a note in the 
> tickets about this. There is no hurry. 
>
> If pushing these patches without reviews seems like a bad idea to you, 
> then please say so (preferably before I commit anything)... 
>
> I am sorry if I haven't worked on other patches I thought I had time 
> to work on. The core ORM refactorings are IMO really important to work 
> on, and thus they have bypassed some other items in my admittedly too 
> long TODO list. 
>
>  - Anssi 
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To view this discussion on the web visit 
https://groups.google.com/d/msg/django-developers/-/6ugSpITwVIEJ.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to 
[email protected].
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

Reply via email to