Inconsistent pagination when sorting by non-unique columns (ticket 34251)

2023-01-11 Thread tatari...@gmail.com
As described in https://code.djangoproject.com/ticket/34251, some database 
backends (incl. PostgreSQL) will produce non-deterministic ordering when 
sorting by columns with non-unique values.
This leads to inconsistent pagination, which I argue falls under the same 
category as paginating by unordered QuerySet. 

I suggest we add a warning similar to the UnorderedObjectListWarning when 
total deterministic ordering is not specified (PK or unique/unique_together 
not null fields).

The consequence of this change is that *a lot* of projects will receive a 
warning. This issue is easy to overlook since non-unique columns like 
"name" can have mostly unique values and you need to have duplicates at the 
page borders to get into trouble. While seemingly hard to reproduce, I 
believe it impacts a lot of production systems out there.

Correctly handling it requires some thought, especially when exposing 
sorting options to the end user. Whether 3rd-party tools like 
django-sortable-listview 
 or DRF 
 (OrderingFilter) should 
automatically enforce the correct ordering by adding "pk"? This brings 
problems with indexing like in this ticket 
. Another option with these 
tools is adding *pk* or another unique field to the query string when 
specifying desired sorting, which looks just wrong to me, pushing 
responsibility to the front end.

What do you folks think? I've been bitten by this issue several times, in a 
production environment after smoothly running for a while. The first time 
was a complete surprise to me, and I believe it's may surprise quite a lot 
of developers as well, so adding a warning will at least bring the 
attention. 

-- 
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/fe395ca6-584e-460c-b533-6853cddcd1f0n%40googlegroups.com.


Re: Inconsistent pagination when sorting by non-unique columns (ticket 34251)

2023-01-11 Thread tatari...@gmail.com
Thanks for the input, Barry!
To be clear - Django already enforces total ordering in the admin panel by 
adding "pk" (in this method 
<https://github.com/django/django/blob/main/django/contrib/admin/views/main.py#L390>),
 
which makes me believe that's a viable approach.
Sure, there can be domain-specific unique/unique_together fields that 
ensure consistent ordering as well, that's why it first checks to see if 
those were used.
What is your solution? Does your frontend/UI provide the "stocking 
number"/"stocking number and store" as the final parameter(s) or do you 
somehow inject them in the VIew? 
P.S. I believe `warnings.filterwarnings` can be used to suppress specific 
warnings. 
On Wednesday, 11 January 2023 at 16:59:41 UTC+2 bajo...@epicor.com wrote:

>
> I'd be very much -1 if the framework modified my sorting parameters (by 
> adding 'pk' at the end of the ordering).  As mentioned, it can certainly 
> break queries that can be handled by a covering index...  but mostly 
> because 'pk' isn't the field we would prefer to use in this case anyway. 
>  For example, when sorting a list of products, the final parameter ought to 
> be the product's stocking number; when looking at items by store, the final 
> parameter needs to be both stocking number and store.  There wouldn't seem 
> to be -any- meaningful "always right" answer.
>
> We have something like 20,000 tests running today (and will probably hit 
> 40,000 over time), many of which do pagination and sorting.   I'd really, 
> really hate to have to go fix all of those tests -- it would be thousands 
> of them.  If there was a way to disable this proposed warning, I could live 
> with that for a while.
>
> baj
> -
> Barry Johnson
>
> On Wednesday, January 11, 2023 at 6:55:25 AM UTC-6 tatari...@gmail.com 
> wrote:
>
>> As described in https://code.djangoproject.com/ticket/34251, some 
>> database backends (incl. PostgreSQL) will produce non-deterministic 
>> ordering when sorting by columns with non-unique values.
>> This leads to inconsistent pagination, which I argue falls under the same 
>> category as paginating by unordered QuerySet. 
>>
>> I suggest we add a warning similar to the UnorderedObjectListWarning when 
>> total deterministic ordering is not specified (PK or unique/unique_together 
>> not null fields).
>>
>> The consequence of this change is that *a lot* of projects will receive 
>> a warning. This issue is easy to overlook since non-unique columns like 
>> "name" can have mostly unique values and you need to have duplicates at the 
>> page borders to get into trouble. While seemingly hard to reproduce, I 
>> believe it impacts a lot of production systems out there.
>>
>> Correctly handling it requires some thought, especially when exposing 
>> sorting options to the end user. Whether 3rd-party tools like 
>> django-sortable-listview 
>> <https://github.com/aptivate/django-sortable-listview> or DRF 
>> <https://github.com/encode/django-rest-framework> (OrderingFilter) 
>> should automatically enforce the correct ordering by adding "pk"? This 
>> brings problems with indexing like in this ticket 
>> <https://code.djangoproject.com/ticket/29943>. Another option with these 
>> tools is adding *pk* or another unique field to the query string when 
>> specifying desired sorting, which looks just wrong to me, pushing 
>> responsibility to the front end.
>>
>> What do you folks think? I've been bitten by this issue several times, in 
>> a production environment after smoothly running for a while. The first time 
>> was a complete surprise to me, and I believe it's may surprise quite a lot 
>> of developers as well, so adding a warning will at least bring the 
>> attention. 
>>
>

-- 
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/91011f02-b18b-47b0-b117-0596d113e82cn%40googlegroups.com.