[Django] #31187: ChangeList._get_deterministic_ordering() should inspect UniqueConstraint

10 views
Skip to first unread message

Django

unread,
Jan 20, 2020, 11:41:30 AM1/20/20
to django-...@googlegroups.com
#31187: ChangeList._get_deterministic_ordering() should inspect UniqueConstraint
---------------------------------------------+------------------------
Reporter: Fabio Sangiovanni | Owner: nobody
Type: New feature | Status: new
Component: contrib.admin | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
---------------------------------------------+------------------------
I've been fiddling with db indexes lately to improve the performance of an
admin view.
Eventually I found this PR https://github.com/django/django/pull/10692
which ensures the records displayed by `ChangeList` are deterministically
ordered.
Among other things, the code looks for the presence of a `unique_together`
attribute on the `Meta` class of the model to determine if the model is
already totally ordered.

I think that `_get_deterministic_ordering()` should check for the presence
of `UniqueConstraints` as well, which currently aren’t considered by the
discovery algorithm.

I noticed the docs currently advise against using `unique_together` in
favor of `UniqueConstraint`, suggesting the former may be deprecated in
the future, so this fix would change that part of the code accordingly.

I'm willing to submit a PR for this.

Cheers,
Fabio

--
Ticket URL: <https://code.djangoproject.com/ticket/31187>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
Jan 20, 2020, 1:25:19 PM1/20/20
to django-...@googlegroups.com
#31187: ChangeList._get_deterministic_ordering() should inspect UniqueConstraint
-------------------------------------+-------------------------------------
Reporter: Fabio Sangiovanni | Owner: Fabio
| Sangiovanni
Type: New feature | Status: assigned
Component: contrib.admin | Version: master
Severity: Normal | Resolution:

Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Fabio Sangiovanni):

* owner: nobody => Fabio Sangiovanni
* status: new => assigned


--
Ticket URL: <https://code.djangoproject.com/ticket/31187#comment:1>

Django

unread,
Jan 20, 2020, 1:51:28 PM1/20/20
to django-...@googlegroups.com
#31187: Detection of existing total ordering in admin changelist should take into
account UniqueConstraints without conditions.

-------------------------------------+-------------------------------------
Reporter: Fabio Sangiovanni | Owner: Fabio
| Sangiovanni
Type: Bug | Status: assigned
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by felixxm):

* type: New feature => Bug
* stage: Unreviewed => Accepted


Comment:

Agreed, we need to remember to ignore `UniqueConstraint`'s with
`condition`'s.

--
Ticket URL: <https://code.djangoproject.com/ticket/31187#comment:2>

Django

unread,
Jan 23, 2020, 12:33:54 PM1/23/20
to django-...@googlegroups.com
#31187: Detection of existing total ordering in admin changelist should take into
account UniqueConstraints without conditions.
-------------------------------------+-------------------------------------
Reporter: Fabio Sangiovanni | Owner: Fabio
| Sangiovanni
Type: Bug | Status: assigned
Component: contrib.admin | Version: master

Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Fabio Sangiovanni):

* has_patch: 0 => 1


Comment:

PR: [https://github.com/django/django/pull/12364]

--
Ticket URL: <https://code.djangoproject.com/ticket/31187#comment:3>

Django

unread,
Feb 19, 2020, 4:18:24 AM2/19/20
to django-...@googlegroups.com
#31187: Detection of existing total ordering in admin changelist should take into
account UniqueConstraints without conditions.
-------------------------------------+-------------------------------------
Reporter: Fabio Sangiovanni | Owner: Fabio
| Sangiovanni
Type: Bug | Status: assigned
Component: contrib.admin | Version: master

Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Mariusz Felisiak <felisiak.mariusz@…>):

In [changeset:"d270c10a723ca04e467e08d6d8b38ae392191f2d" d270c10a]:
{{{
#!CommitTicketReference repository=""
revision="d270c10a723ca04e467e08d6d8b38ae392191f2d"
Refs #31187 -- Added total_unique_constraints property to model's Options.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/31187#comment:4>

Django

unread,
Feb 19, 2020, 4:18:25 AM2/19/20
to django-...@googlegroups.com
#31187: Detection of existing total ordering in admin changelist should take into
account UniqueConstraints without conditions.
-------------------------------------+-------------------------------------
Reporter: Fabio Sangiovanni | Owner: Fabio
| Sangiovanni
Type: Bug | Status: closed
Component: contrib.admin | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Mariusz Felisiak <felisiak.mariusz@…>):

* status: assigned => closed
* resolution: => fixed


Comment:

In [changeset:"b457068cf272bc9912187a2b7aaaa4445110735e" b457068c]:
{{{
#!CommitTicketReference repository=""
revision="b457068cf272bc9912187a2b7aaaa4445110735e"
Fixed #31187 -- Fixed detecting of existing total ordering in admin
changelist when using Meta.constraints.

Detection of existing total ordering in admin changelist now takes into
account non-partial unique constraints.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/31187#comment:5>

Reply all
Reply to author
Forward
0 new messages