[Django] #33543: The behaviour of nulls_first and nulls_last is surprising and confusing.
Django
-------------------------------------+-------------------------------------
Reporter: Gordon | Owner: nobody
Wrigley |
Type: | Status: new
Uncategorized |
Component: Database | Version: 3.2
layer (models, ORM) |
Severity: Normal | Keywords:
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
Consider the following:
{{{
In [11]: [tv.published_at for tv in
TemplateVersion.objects.order_by(F("published_at").desc(nulls_first=True))]
Out[11]:
[None,
datetime.datetime(2022, 2, 25, 13, 0, 12, 91916, tzinfo=<UTC>),
datetime.datetime(2022, 2, 21, 10, 18, 0, 169248, tzinfo=<UTC>)]
In [12]: [tv.published_at for tv in
TemplateVersion.objects.order_by(F("published_at").desc(nulls_first=False))]
Out[12]:
[None,
datetime.datetime(2022, 2, 25, 13, 0, 12, 91916, tzinfo=<UTC>),
datetime.datetime(2022, 2, 21, 10, 18, 0, 169248, tzinfo=<UTC>)]
In [13]: [tv.published_at for tv in
TemplateVersion.objects.order_by(F("published_at").desc(nulls_last=True))]
Out[13]:
[datetime.datetime(2022, 2, 25, 13, 0, 12, 91916, tzinfo=<UTC>),
datetime.datetime(2022, 2, 21, 10, 18, 0, 169248, tzinfo=<UTC>),
None]
In [14]: [tv.published_at for tv in
TemplateVersion.objects.order_by(F("published_at").desc(nulls_last=False))]
Out[14]:
[None,
datetime.datetime(2022, 2, 25, 13, 0, 12, 91916, tzinfo=<UTC>),
datetime.datetime(2022, 2, 21, 10, 18, 0, 169248, tzinfo=<UTC>)]
}}}
Observe how `nulls_first=False` still puts the nulls first.
This happens because they both default False and when they are both False
it lets the DB decide.
This is surprising behaviour, it also makes changing the null positioning
based on a variable more awkward than it needs to be.
I think it would be better if they defaulted to None, let the DB decide
when both are None and when one is not None do the ordering that implies.
--
Ticket URL: <https://code.djangoproject.com/ticket/33543>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Type: Uncategorized | Status: new
Component: Database layer | Version: 3.2
(models, ORM) |
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
-------------------------------------+-------------------------------------
Comment (by Adam Johnson):
I concur that it is confusing that `nulls_first=False` can still put the
nulls first, and equally for the `nulls_last=False`.
I don't think we can change the semantics though as it would be a breaking
change. The best we can probably do is make passing `False` for either a
`TypeError`, by swapping their defaults for sentinel values.
Then, if you need a variable to switch between the behaviours you can use
a construct like:
{{{
F(...).desc(**{("nulls_first" if nulls_first else "nulls_last"): True})
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/33543#comment:1>
Django
-------------------------------------+-------------------------------------
Reporter: Gordon Wrigley | Owner:
Cleanup/optimization | Status: assigned
Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: nobody => AllenJonathan
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/33543#comment:3>
Django
-------------------------------------+-------------------------------------
Reporter: Gordon Wrigley | Owner:
Type: | AllenJonathan
Cleanup/optimization | Status: assigned
Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
* has_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/15632 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/33543#comment:4>
Django
-------------------------------------+-------------------------------------
Reporter: Gordon Wrigley | Owner:
Type: | AllenJonathan
Cleanup/optimization | Status: assigned
Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 1 => 0
Comment:
[https://github.com/django/django/pull/15682 New PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/33543#comment:5>
Django
-------------------------------------+-------------------------------------
Reporter: Gordon Wrigley | Owner:
Type: | AllenJonathan
Component: Database layer | Version: 3.2
(models, ORM) |
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"68da6b389c403cb91650754be0e2287696807333" 68da6b3]:
{{{
#!CommitTicketReference repository=""
revision="68da6b389c403cb91650754be0e2287696807333"
Fixed #33543 -- Deprecated passing nulls_first/nulls_last=False to OrderBy
and Expression.asc()/desc().
Thanks Allen Jonathan David for the initial patch.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/33543#comment:6>
Django
-------------------------------------+-------------------------------------
Reporter: Gordon Wrigley | Owner:
Type: | AllenJonathan
Cleanup/optimization | Status: closed
Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"94ad46e9d8077d8fadce991af85be657b4a4e2a0" 94ad46e9]:
{{{
#!CommitTicketReference repository=""
revision="94ad46e9d8077d8fadce991af85be657b4a4e2a0"
Refs #33543 -- Made Expression.asc()/desc() and OrderBy raise ValueError
when nulls_first/nulls_last=False is passed.
Per deprecation timeline.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/33543#comment:7>