Generative values() Does not Work

[mtrier]

Before opening a ticket I thought I would check what the expected
behavior is for generative values(). This is the behavior I'm seeing
on Queryset Refactor branch and to me it seems like it is not the
correct behavior.

>>> from original.foo.models import Category
>>> Category.objects.all()
[<Category: Algol>, <Category: Smalltalk>, <Category: PHP>, <Category:
CSharp>]
>>> Category.objects.values('name')
[{'name': u'Algol'}, {'name': u'Smalltalk'}, {'name': u'PHP'},
{'name': u'CSharp'}]
>>> Category.objects.values('name', 'id')
[{'name': u'Algol', 'id': 1}, {'name': u'Smalltalk', 'id': 2},
{'name': u'PHP', 'id': 3}, {'name': u'CSharp', 'id': 4}]
>>> Category.objects.values('name').values('id')
[{'name': u'Algol'}, {'name': u'Smalltalk'}, {'name': u'PHP'},
{'name': u'CSharp'}]

It would seem to me that the last query should return the same thing
as the prior one.

Michael Trier
blog.michaeltrier.com

[Russell Keith-Magee]

Agreed. This looks like a bug to me; at a guess, the problem will be
that when ValuesQuerySet is cloned, it doesn't copy the attribute list.

It may be worth checking if this is still a problem with queryset-
refactor; if it is stil a problem, it might be better to fix the
problem there, rather than trunk.

Yours,
Russ Magee %-)

[Malcolm Tredinnick]

Whoops.. I missed this a couple of days ago. I think I lost
consciousness somewhere in the Py3k thread when catching up after the
weekend.

On Tue, 2008-04-01 at 20:45 +0800, Russell Keith-Magee wrote:
>
> On 31/03/2008, at 2:33 AM, mtrier wrote:

[...]

> This is the behavior I'm seeing
> > on Queryset Refactor branch and to me it seems like it is not the
> > correct behavior.

[...]

> >>>> Category.objects.values('name', 'id')
> > [{'name': u'Algol', 'id': 1}, {'name': u'Smalltalk', 'id': 2},
> > {'name': u'PHP', 'id': 3}, {'name': u'CSharp', 'id': 4}]
> >>>> Category.objects.values('name').values('id')
> > [{'name': u'Algol'}, {'name': u'Smalltalk'}, {'name': u'PHP'},
> > {'name': u'CSharp'}]
> >
> > It would seem to me that the last query should return the same thing
> > as the prior one.
>
> Agreed. This looks like a bug to me; at a guess, the problem will be
> that when ValuesQuerySet is cloned, it doesn't copy the attribute list.
>
> It may be worth checking if this is still a problem with queryset-
> refactor; if it is stil a problem, it might be better to fix the
> problem there, rather than trunk.

Michael's using the branch (see the first quoted section).

It's a bug. I'll fix it.

Malcolm

--
If you think nobody cares, try missing a couple of payments.
http://www.pointy-stick.com/blog/

[Russell Keith-Magee]

On 4/2/08, Malcolm Tredinnick <mal...@pointy-stick.com> wrote:
>
> > It may be worth checking if this is still a problem with queryset-
> > refactor; if it is stil a problem, it might be better to fix the
> > problem there, rather than trunk.
>
>
> Michael's using the branch (see the first quoted section).

Note to self: Read the message first. :-)

Russ %-)

[Gary Wilson Jr.]

Russell Keith-Magee wrote:
>
> On 31/03/2008, at 2:33 AM, mtrier wrote:
>
>> Before opening a ticket I thought I would check what the expected
>> behavior is for generative values(). This is the behavior I'm seeing
>> on Queryset Refactor branch and to me it seems like it is not the
>> correct behavior.
>>
>>>>> from original.foo.models import Category
>>>>> Category.objects.all()
>> [<Category: Algol>, <Category: Smalltalk>, <Category: PHP>, <Category:
>> CSharp>]
>>>>> Category.objects.values('name')
>> [{'name': u'Algol'}, {'name': u'Smalltalk'}, {'name': u'PHP'},
>> {'name': u'CSharp'}]
>>>>> Category.objects.values('name', 'id')
>> [{'name': u'Algol', 'id': 1}, {'name': u'Smalltalk', 'id': 2},
>> {'name': u'PHP', 'id': 3}, {'name': u'CSharp', 'id': 4}]
>>>>> Category.objects.values('name').values('id')
>> [{'name': u'Algol'}, {'name': u'Smalltalk'}, {'name': u'PHP'},
>> {'name': u'CSharp'}]
>>
>> It would seem to me that the last query should return the same thing
>> as the prior one.
>
> Agreed. This looks like a bug to me; at a guess, the problem will be
> that when ValuesQuerySet is cloned, it doesn't copy the attribute list.

I also agree that this is a bug, as the "first one wins" behavior displayed
above is not intuitive. But what should the desired behavior should be?

The OP suggests an additive behavior similar to the way filter()
operates. However, there is also the option of a "last one wins" approach, as
seen by the order_by() method (on trunk, is it the same in qs-rf?):

>>> User.objects.order_by('last_name')._order_by
('last_name',)

>>> User.objects.order_by('last_name').order_by('first_name')._order_by
('first_name',)

>>> User.objects.order_by('last_name', 'first_name')._order_by
('last_name', 'first_name')

If I had to pick a behavior, I guess I would lean towards the "last one wins"
approach. order_by() and values() are methods that I typically use towards
the end of my QuerySet constructions, and if I'm passed a QuerySet from some
other code I would like my order_by() or values() to take precedence.

Gary

[mtrier]

> The OP suggests an additive behavior similar to the way filter()
> operates.  However, there is also the option of a "last one wins" approach, as
> seen by the order_by() method (on trunk, is it the same in qs-rf?):

Although, you have a point, I have to say that limitation in order_by
bugs me as well. :)

Michael Trier
blog.michaeltrier.com

[alex....@gmail.com]

I think one of the reasons that order_by only uses the last one is
because you have often have a default that you may want to override,
and I guess the consensus is you shouldn't have to specify to clear
the default ordering if you reorder. Whether that should apply here
I'm not sure.

[Malcolm Tredinnick]

On Thu, 2008-04-03 at 17:53 -0700, mtrier wrote:
> > The OP suggests an additive behavior similar to the way filter()
> > operates. However, there is also the option of a "last one wins" approach, as
> > seen by the order_by() method (on trunk, is it the same in qs-rf?):
>
> Although, you have a point, I have to say that limitation in order_by
> bugs me as well. :)

I think that's a case of learning to live with your disappointment. It
would lead to a lot of counter-intuitive behaviour to make order_by()
incremental, because the *first* ordering condition is the main sorting
column and then equal values are sorted by the second, etc (that's the
way SQL works). So if you *added* an extra order_by(...) constraint it
would have no effect in the current system. We'd have to say that
order_by(...) would be applied in reverse order, which will make
people's brains hurt, because it's entirely non-obvious.

What's a real use-case where you need to sort by multiple columns and
don't know what they all are at once? I don't think there's a case to
say you should be able to add a fifth-most-important sort criteria with
a new order-by() call because although you might remember that's how it
works, nobody else will and people will be wondering why they "can't
change the ordering" (it will appear) when they call order_by().

Regards,
Malcolm

--
If it walks out of your refrigerator, LET IT GO!!
http://www.pointy-stick.com/blog/

[Empty]

> > Although, you have a point, I have to say that limitation in order_by
> > bugs me as well. :)
>
> I think that's a case of learning to live with your disappointment. It
> would lead to a lot of counter-intuitive behaviour to make order_by()
> incremental, because the *first* ordering condition is the main sorting
> column and then equal values are sorted by the second, etc (that's the
> way SQL works). So if you *added* an extra order_by(...) constraint it
> would have no effect in the current system. We'd have to say that
> order_by(...) would be applied in reverse order, which will make
> people's brains hurt, because it's entirely non-obvious.

I apologize that my off-handed comment caused you to go into this much
explanation, but I appreciate it nonetheless. I see where you're
coming from, especially the idea of constraining it further.

Thanks again for clarifying.

Michael Trier
blog.michaeltrier.com