I’d vote for option 2. I don’t think it can be expected the ordering will
be obeyed when not selecting the columns it includes.

On Wed, 28 Oct 2020 at 19:03, Tobias McNulty <tob...@caktusgroup.com> wrote:

> Hi all,
>
> Starting a thread on ticket #28560
> <https://code.djangoproject.com/ticket/28560>, "distinct() on ordered
> queryset with restricted list of columns returns incorrect result." In a
> nutshell:
>
> $ cat testapp/models.py
>
> from django.db import models
>
>
>
> class School(models.Model):
>
>     name = models.CharField(max_length=255)
>
>     county = models.CharField(max_length=255)
>
>
>     class Meta:
>
>         ordering = ("name",)
>
> $ python manage.py shell
>
> ...
>
> >>> from testapp.models import School
>
> >>> str(School.objects.values("county").distinct().query)
>
> 'SELECT DISTINCT "testapp_school"."county", *"testapp_school"."name"*
> FROM "testapp_school" ORDER BY "testapp_school"."name" ASC'
>
> Note the "name" column is added implicitly to the SELECT clause due to the
> default ordering on the model (I believe with good reason, since #7070
> <https://code.djangoproject.com/ticket/7070>). This is not specific to a
> default ordering; it's also possible to generate unintended results with a
> mismatching list of columns between an explicit order_by() and values().
>
> It's possible to fix with an empty order_by():
>
> >>> str(School.objects.values("county").order_by().distinct().query)
>
> 'SELECT DISTINCT "testapp_school"."county" FROM "testapp_school"'
>
>
> But, this still feels like a case where we could do better. Some potential
> options:
>
>    1. It looks like there was an initial attempt to fix the issue with a
>    subquery, but from what I can tell it was not possible to preserve
>    ordering
>    <https://github.com/django/django/pull/9055#issuecomment-338276279> in
>    the outer query.
>    2. My colleague Dmitriy pointed out that there may be a precedent for 
> excluding
>    the default ordering <https://github.com/django/django/pull/10005> for
>    queries like this.
>    3. An option I suggested on the ticket
>    <https://code.djangoproject.com/ticket/28560#comment:13> is to raise
>    an error if the list of columns in values() is insufficient to run the
>    requested query (i.e., never add a column implicitly if the user specified
>    a list of columns via values() or values_list()).
>
> What do others think? What are other potential fixes I'm not thinking of?
>
> Cheers,
>
>
> *Tobias McNulty*Chief Executive Officer
>
> tob...@caktusgroup.com
> www.caktusgroup.com
>
> --
> You received this message because you are subscribed to the Google Groups
> "Django developers (Contributions to Django itself)" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to django-developers+unsubscr...@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/django-developers/CAMGFDKRQsbUz%3DeYKxX2U%2B892AgQbXr%3DMA9AiWw9vfiPcM%2BnAXw%40mail.gmail.com
> <https://groups.google.com/d/msgid/django-developers/CAMGFDKRQsbUz%3DeYKxX2U%2B892AgQbXr%3DMA9AiWw9vfiPcM%2BnAXw%40mail.gmail.com?utm_medium=email&utm_source=footer>
> .
>
-- 
Adam

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/CAMyDDM2Sdhgyb1e19UbCKrJDAaPepT4nOiD_4MGjb-qhVyxmOw%40mail.gmail.com.

Reply via email to