Re: M2M with to_fields (ticket #18823)

2012-08-27 Thread Russell Keith-Magee
On Tue, Aug 28, 2012 at 3:11 AM, peter  wrote:
> I opened this ticket (https://code.djangoproject.com/ticket/18823) on the
> Trac but thought i'd bring it up here to increase the likelihood of it
> getting noticed. In short things don't quite work right when you have a m2m
> field that uses a through model that uses a to_field for it's foreign key.
> If someone wouldn't mind reviewing the patch and giving me some feedback,
> I'd like to help get this issue fixed.
>
> Assuming a fix in some form is accepted, I was also wondering if this bug
> fix would be considered important enough to backport to bugfix branches? Or
> will we need to wait for the next major release to get the fix in an
> official release.

Generally speaking, we don't backport bug fixes unless they relate to:

 * Security issues

 * Major problems with newly added features

 * Problems with the potential to cause catastrophic data los -- i.e.,
accidentally deleting/dropping data, or deleting/dropping the wrong
data.

A rule of thumb: If we'd discovered this feature the day before we cut
a major release, would we have held back the release?

In this case, it's not a security issue, and the code has had this
problem for several years; we certainly aim for being bug free, but
we've evidently survived with this bug for some time without any
serious problems. However, you *might* be able to make the case that
catastrophic data loss is possible. From an initial analysis, it seems
like it might be possible to accidentally delete the wrong through
model. If you can provide a clear example of a case where this would
happen, then yes, a backport is likely.

Yours,
Russ Magee %-)

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



M2M with to_fields (ticket #18823)

2012-08-27 Thread peter
I opened this ticket (https://code.djangoproject.com/ticket/18823) on the 
Trac but thought i'd bring it up here to increase the likelihood of it 
getting noticed. In short things don't quite work right when you have a m2m 
field that uses a through model that uses a to_field for it's foreign 
key. If someone wouldn't mind reviewing the patch and giving me some 
feedback, I'd like to help get this issue fixed.

Assuming a fix in some form is accepted, I was also wondering if this bug 
fix would be considered important enough to backport to bugfix branches? Or 
will we need to wait for the next major release to get the fix in 
an official release.

Thanks for your time.

-- 
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/-/hHZ8EqEv_msJ.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: ORM refactoring, part 2

2012-08-27 Thread Luke Plant
Anssi,

I just got back from holiday. I hope to be able to review at least some
of these patches within the next 2 weeks. If you don't hear from me in
that time, I'd encourage you to carry on anyway.

Thanks,

Luke

On 25/08/12 20:35, 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
> 


-- 
Sometimes I wonder if men and women really suit each other. Perhaps
they should live next door and just visit now and then. (Katherine
Hepburn)

Luke Plant || http://lukeplant.me.uk/

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit