[Django] #31573: QuerySet.update().order_by() not working as expected on MySQL
Django
-------------------------------------+-------------------------------------
Reporter: Gerben | Owner: nobody
Morsink |
Type: Bug | Status: new
Component: Database | Version: 2.2
layer (models, ORM) |
Severity: Normal | Keywords: queryset update
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
When a UniqueConstraint is used on an IntegerField and we want to update
multiple rows of that field at once, we need to make sure we select the
right order before the update, since MySQL does not support a deferred
update (see: https://dev.mysql.com/doc/refman/5.7/en/update.html).
I expected simply doing:
qs.order_by('-integerfield').update(integerfield=F('integerfield')+1)
would work, but it seems the Django ORM strips the order_by from the query
before execution, it gives a django.db.utils.IntegrityError: (1062,
"Duplicate entry '' for key 'integerfield'").
--
Ticket URL: <https://code.djangoproject.com/ticket/31573>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Reporter: Gerben Morsink | Owner: nobody
Type: New feature | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: update mysql order | Triage Stage: Accepted
order_by |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* keywords: queryset update => update mysql order order_by
* type: Bug => New feature
* version: 2.2 => master
* stage: Unreviewed => Accepted
Comment:
MySQL seems to be only supported engine to allow `UPDATE ... ORDER BY` so
it doesn't seem to be standard.
- https://www.postgresql.org/docs/current/sql-update.html
- https://www.sqlite.org/draft/lang_update.html
-
https://docs.oracle.com/cd/B19306_01/appdev.102/b14261/update_statement.htm
Since it's the documented way of dealing with the lack of deferrable
unique constraint on MySQL I guess we could add support for it.
--
Ticket URL: <https://code.djangoproject.com/ticket/31573#comment:1>
Django
-------------------------------------+-------------------------------------
Reporter: Gerben Morsink | Owner: nobody
Type: New feature | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: update mysql order | Triage Stage: Accepted
order_by |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Simon Charette):
For anyone interested in implementing this feature you'll have to override
the `as_sql` and `pre_sql_setup` method of
`django.db.models.sql.compiler.SQLUpdateCompiler` in
`django.db.backends.mysql.compiler.SQLUpdateCompiler` to
[https://github.com/django/django/blob/d51e090db2110f016dbca1d794c0d379b3df551b/django/db/models/sql/compiler.py#L1535
avoid clearing the ordering] and append the `ORDER BY` clause to
`super().as_sql()[0]`.
--
Ticket URL: <https://code.djangoproject.com/ticket/31573#comment:2>
Django
-------------------------------------+-------------------------------------
Reporter: Gerben Morsink | Owner: nobody
Type: New feature | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: update mysql order | Triage Stage: Accepted
order_by |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: Adam (Chainz) Johnson (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/31573#comment:3>
Django
-------------------------------------+-------------------------------------
Reporter: Gerben Morsink | Owner: nobody
Type: New feature | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: update mysql order | Triage Stage: Accepted
order_by |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Adam (Chainz) Johnson):
I imagine we'd also want a DB feature flag `supports_update_order_by` to
gate this behaviour.
--
Ticket URL: <https://code.djangoproject.com/ticket/31573#comment:4>
Django
-------------------------------------+-------------------------------------
| Chorpash
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: update mysql order | Triage Stage: Accepted
order_by |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: nobody => David Chorpash
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/31573#comment:5>
Django
-------------------------------------+-------------------------------------
Reporter: Gerben Morsink | Owner: David
| Chorpash
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: update mysql order | Triage Stage: Accepted
order_by |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by David Chorpash):
I would like to give a quick update.
I am able to get the following to work with MySQL on a unique constraint
field:
{{{
QuerySet.order_by().update()
}}}
but I am still working on
{{{
QuerySet.update().order_by()
}}}
I'm also unsure how to add a feature flag here (I'm still new). I'm not
sure if I just missed it in the documentation, but if anyone could point
me in the right direction, that would be greatly appreciated!
--
Ticket URL: <https://code.djangoproject.com/ticket/31573#comment:6>
Django
-------------------------------------+-------------------------------------
Reporter: Gerben Morsink | Owner: David
| Chorpash
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: update mysql order | Triage Stage: Accepted
order_by |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Adam (Chainz) Johnson):
I don't think it's possible to support `.update().order_by()` because
`update()` doesn't return another queryset, but instead actually does the
update and returns the number of rows. Supporting only the
`.order_by().update()` syntax seems fine to me.
--
Ticket URL: <https://code.djangoproject.com/ticket/31573#comment:7>
Django
-------------------------------------+-------------------------------------
Reporter: Gerben Morsink | Owner: David
| Chorpash
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: update mysql order | Triage Stage: Accepted
order_by |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by David Chorpash):
Replying to [comment:7 Adam (Chainz) Johnson]:
> I don't think it's possible to support `.update().order_by()` because
`update()` doesn't return another queryset, but instead actually does the
update and returns the number of rows. Supporting only the
`.order_by().update()` syntax seems fine to me.
Thanks for the info!
I added the following [https://github.com/django/django/pull/13088 PR]
which allows for the use of `.update().order_by()`.
--
Ticket URL: <https://code.djangoproject.com/ticket/31573#comment:8>
Django
-------------------------------------+-------------------------------------
Reporter: Gerben Morsink | Owner: David
| Chorpash
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: update mysql order | Triage Stage: Accepted
order_by |
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/31573#comment:9>
Django
-------------------------------------+-------------------------------------
Reporter: Gerben Morsink | Owner: David
| Chorpash
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: update mysql order | Triage Stage: Accepted
order_by |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 1 => 0
Comment:
Addressed comments in the PR. Added more test cases to handle a wider
variety of use-cases.
--
Ticket URL: <https://code.djangoproject.com/ticket/31573#comment:10>
Django
-------------------------------------+-------------------------------------
Reporter: Gerben Morsink | Owner: David
| Chorpash
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: update mysql order | Triage Stage: Accepted
order_by |
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
* needs_docs: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/31573#comment:11>
Django
-------------------------------------+-------------------------------------
Reporter: Gerben Morsink | Owner: David
| Chorpash
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: update mysql order | Triage Stage: Accepted
order_by |
Has patch: 1 | Needs documentation: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/31573#comment:12>
Django
-------------------------------------+-------------------------------------
Reporter: Gerben Morsink | Owner: David
| Chorpash
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
order_by | checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_docs: 1 => 0
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/31573#comment:13>
Django
-------------------------------------+-------------------------------------
Reporter: Gerben Morsink | Owner: David
| Chorpash
Component: Database layer | Version: master
(models, ORM) |
Keywords: update mysql order | Triage Stage: Ready for
order_by | checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"779e615e362108862f1681f965ee9e4f1d0ae6d2" 779e615e]:
{{{
#!CommitTicketReference repository=""
revision="779e615e362108862f1681f965ee9e4f1d0ae6d2"
Fixed #31573 -- Made QuerySet.update() respect ordering on MariaDB/MySQL.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/31573#comment:14>