The excuse at the time was that we added QuerySet.exclude() and that made "ne" lookups redundant. Problem is, that's technically not true, because "ne" generated SQL that was more efficient than what exclude() currently generates. Specifically, the problem is that the exclude() SQL doesn't take advantage of indexes on mycolumn as efficiently as the old-style lookup. Here's the difference:
-- Old-style ne lookup SELECT * FROM mytable WHERE mycolumn != 'foo';
-- exclude() SELECT * FROM mytable WHERE NOT (mycolumn = 'foo');
I'd like for Django to go back to the more efficient query here, instead of the lame "NOT" query. Two solutions come to mind: we can restore the "ne" lookup type, or we can change the exclude() implementation to generate "!=" in the underlying SQL if possible. The advantage of the former is that it's much simpler to implement, but the advantage of the latter is that any code currently using exclude() would get the benefit of faster queries. (Note that I imagine only people using exclude() with a single "exact" lookup parameter would get the benefit; we could get trickier beyond that, but...diminishing returns.) We could also do both.
I'm inclined to say we do the former -- restore the "ne" lookup type -- because it's a quick fix, and ask somebody to write up a patch for the latter. Does anybody have strong opinions against this? If not, I can restore the "ne" lookup type.
On Wed, Oct 27, 2010 at 4:32 PM, Adrian Holovaty <adr...@holovaty.com> wrote: > I'm inclined to say we do the former -- restore the "ne" lookup type > -- because it's a quick fix, and ask somebody to write up a patch for > the latter. Does anybody have strong opinions against this? If not, I > can restore the "ne" lookup type.
Sounds like a good plan to me (especially making simple excludes faster).
However, just for the record I think the reason we decided to remove __ne is the first place was that its existence introduces a weird inconsistency with regard to other lookup types. That is, if there's a "ne" why isn't there a "nstartswith" or "nrange" or ... ? I think down that path lies madness so I'm +0 on bringing back "ne" with the proviso that we agree it's not the first step down a slippery slope towards "nistartswith" and friends.
On Wed, Oct 27, 2010 at 5:55 PM, Jacob Kaplan-Moss <ja...@jacobian.org> wrote: > On Wed, Oct 27, 2010 at 4:32 PM, Adrian Holovaty <adr...@holovaty.com> wrote: >> I'm inclined to say we do the former -- restore the "ne" lookup type >> -- because it's a quick fix, and ask somebody to write up a patch for >> the latter. Does anybody have strong opinions against this? If not, I >> can restore the "ne" lookup type.
> Sounds like a good plan to me (especially making simple excludes faster).
> However, just for the record I think the reason we decided to remove > __ne is the first place was that its existence introduces a weird > inconsistency with regard to other lookup types. That is, if there's a > "ne" why isn't there a "nstartswith" or "nrange" or ... ? I think down > that path lies madness so I'm +0 on bringing back "ne" with the > proviso that we agree it's not the first step down a slippery slope > towards "nistartswith" and friends.
> Jacob
> -- > 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+unsubscribe@googlegroups.com. > For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
I'd be -0.5 (let's see how pusillanimous I can be!) on introducing __ne, but +1 on fixing the internals to generate proper not equal comparisons.
Alex
-- "I disapprove of what you say, but I will defend to the death your right to say it." -- Voltaire "The people's good is the highest law." -- Cicero "Code can always be simpler than you think, but never as simple as you want" -- Me
On Wed, Oct 27, 2010 at 4:55 PM, Jacob Kaplan-Moss <ja...@jacobian.org> wrote: > However, just for the record I think the reason we decided to remove > __ne is the first place was that its existence introduces a weird > inconsistency with regard to other lookup types. That is, if there's a > "ne" why isn't there a "nstartswith" or "nrange" or ... ? I think down > that path lies madness so I'm +0 on bringing back "ne" with the > proviso that we agree it's not the first step down a slippery slope > towards "nistartswith" and friends.
Totally agree we should set clear expectations that "ne" does not mean we'll be getting nistartswith or any other negative lookup -- good call in bringing that up. This should be treated as a special case for a common case.
On Oct 27, 5:55 pm, Jacob Kaplan-Moss <ja...@jacobian.org> wrote:
> On Wed, Oct 27, 2010 at 4:32 PM, Adrian Holovaty <adr...@holovaty.com> wrote:
> > I'm inclined to say we do the former -- restore the "ne" lookup type
> > -- because it's a quick fix, and ask somebody to write up a patch for
> > the latter. Does anybody have strong opinions against this? If not, I
> > can restore the "ne" lookup type.
> Sounds like a good plan to me (especially making simple excludes faster).
> However, just for the record I think the reason we decided to remove
> __ne is the first place was that its existence introduces a weird
> inconsistency with regard to other lookup types. That is, if there's a
> "ne" why isn't there a "nstartswith" or "nrange" or ... ? I think down
> that path lies madness so I'm +0 on bringing back "ne" with the
> proviso that we agree it's not the first step down a slippery slope
> towards "nistartswith" and friends.
I know it's been a little while since I've made any major ORM
contributions, but I'd say -0 on __ne, and +1 on making exclude
generate better code. Django's worked far too hard on making things
consistent as possible to let like this slip by just because we don't
want to muddy our hands with a little harder work in the exclude()
code. So many other tickets have been stuck in DDN/Accepted forever
because the area of code is harder to review, it's not like it's an
unknown state in the project. :)
I'd even be willing to throw my hat in the ring to contribute towards
an .exclude()-based solution if someone else doesn't step forward, but
I know I won't be touching it until a few days pass.
On Oct 27, 2010, at 5:55 PM, Jacob Kaplan-Moss wrote:
> On Wed, Oct 27, 2010 at 4:32 PM, Adrian Holovaty <adr...@holovaty.com> wrote: >> I'm inclined to say we do the former -- restore the "ne" lookup type >> -- because it's a quick fix, and ask somebody to write up a patch for >> the latter. Does anybody have strong opinions against this? If not, I >> can restore the "ne" lookup type.
> Sounds like a good plan to me (especially making simple excludes faster).
> However, just for the record I think the reason we decided to remove > __ne is the first place was that its existence introduces a weird > inconsistency with regard to other lookup types. That is, if there's a > "ne" why isn't there a "nstartswith" or "nrange" or ... ? I think down > that path lies madness so I'm +0 on bringing back "ne" with the > proviso that we agree it's not the first step down a slippery slope > towards "nistartswith" and friends.
I know it's been a little while since I've made any major ORM contributions, but I'd say -0 on __ne, and +1 on making exclude generate better code. Django's worked far too hard on making things consistent as possible to let like this slip by just because we don't want to muddy our hands with a little harder work in the exclude() code. So many other tickets have been stuck in DDN/Accepted forever because the area of code is harder to review, it's not like it's an unknown state in the project. :)
I'd even be willing to throw my hat in the ring to contribute towards an .exclude()-based solution if someone else doesn't step forward, but I know I won't be touching it until a few days pass.
On Thu, Oct 28, 2010 at 6:59 AM, George Vilches <g...@thataddress.com> wrote:
> On Oct 27, 5:55 pm, Jacob Kaplan-Moss <ja...@jacobian.org> wrote: >> On Wed, Oct 27, 2010 at 4:32 PM, Adrian Holovaty <adr...@holovaty.com> wrote: >> > I'm inclined to say we do the former -- restore the "ne" lookup type >> > -- because it's a quick fix, and ask somebody to write up a patch for >> > the latter. Does anybody have strong opinions against this? If not, I >> > can restore the "ne" lookup type.
>> Sounds like a good plan to me (especially making simple excludes faster).
>> However, just for the record I think the reason we decided to remove >> __ne is the first place was that its existence introduces a weird >> inconsistency with regard to other lookup types. That is, if there's a >> "ne" why isn't there a "nstartswith" or "nrange" or ... ? I think down >> that path lies madness so I'm +0 on bringing back "ne" with the >> proviso that we agree it's not the first step down a slippery slope >> towards "nistartswith" and friends.
> I know it's been a little while since I've made any major ORM > contributions, but I'd say -0 on __ne, and +1 on making exclude > generate better code. Django's worked far too hard on making things > consistent as possible to let like this slip by just because we don't > want to muddy our hands with a little harder work in the exclude() > code.
I agree. We should certainly fix the behavior of exclude(), but reintroducing __ne feels like the start of a slippery slope to me. Conceptually, I'm -1 on reintroducing __ne.
In the interests of practicality, I'd be willing to reconsider if it turns out that fixing exclude() is prohibitively complex. However, it seems premature to reintroduce __ne as a stop-gap measure before anyone has taken a serious look at what is involved with fixing exclude(). We still have almost 3 months until the freeze for 1.3; that should be plenty of time to investigate exactly how much work would be involved in fixing this bug properly.