[Django] #37097: Nested double-unioned query for model with ordering fails on Postgres/MySql

[Django]

#37097: Nested double-unioned query for model with ordering fails on Postgres/MySql
-------------------------------------+-------------------------------------
Reporter: Shai Berger | Type: Bug
Status: new | Component: Database
| layer (models, ORM)
Version: dev | Severity: Release
| blocker
Keywords: Regression | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Yep. Sorry about the title, that's the most succinct I can make it.

Add the following test in
{{{django/tests/queries/test_qs_combinators.py}}}, say after
{{{test_union_in_with_ordering_and_slice}}}

{{{#!python
def test_double_union_in_with_ordering(self):
qs1 = Author.objects.filter(num__gt=7)
qs2 = Author.objects.filter(num__lt=2)
qs3 = Author.objects.filter(num=5)
self.assertQuerySetEqual(
# Query formatted to allow easy commenting-out of parts
Author.objects.exclude(id__in=qs1
.union(qs2, all=True)
.union(qs3, all=True)
.values("id")),
[]
)
}}}

On Postgres, this passes on 6.0, but fails on the main branch (towards 6.1
as I write this). The error is
{{{
django.db.utils.ProgrammingError: each UNION query must have the same
number of columns
LINE 1: ...) ORDER BY "__orderbycol2" ASC) UNION ALL (SELECT "U0"."id" ...
}}}
because the query (formatted for some readability) is
{{{#!sql
SELECT "queries_author"."id", "queries_author"."name",
"queries_author"."num", "queries_author"."extra_id"
FROM "queries_author"
WHERE NOT (
"queries_author"."id" IN (
(
(
SELECT "U0"."id" AS "id", "U0"."name" AS "__orderbycol2"
FROM "queries_author" "U0"
WHERE "U0"."num" > 7
ORDER BY "U0"."name" ASC
) UNION ALL (
SELECT "U0"."id" AS "id", "U0"."name" AS "__orderbycol2"
FROM "queries_author" "U0"
WHERE "U0"."num" < 2
ORDER BY "U0"."name" ASC
)
ORDER BY "__orderbycol2" ASC -- I think this is the
culprit
) UNION ALL (
SELECT "U0"."id" AS "id"
FROM "queries_author" "U0"
WHERE "U0"."num" = 5
ORDER BY "U0"."name" ASC
)
)
)
ORDER BY "queries_author"."name" ASC
}}}

Notes:
- Double union is required, a single union works
- This happens when the ordering is defined in the model Meta, but not if
you just add ordering on the queries. I suspect this is because having the
ordering in the Meta makes the query equivalent to
{{{#!python
Author.objects.exclude(id__in=qs1
.union(qs2, all=True)
.order_by("name") # This is implied
.union(qs3, all=True)
.values("id"))
}}}
- On Oracle and Sqlite, the inner order is removed by the fix of #36938. I
haven't actually tested it on MySql, but I believe it has the same
relevant feature flags and so should behave the same.
- The test as written fails on Sqlite on 6.0, with
{{{django.db.utils.DatabaseError: ORDER BY not allowed in subqueries of
compound statements}}} -- similar tests are skipped there; but it passes
on Postgres, and passes on Sqlite on main.
--
Ticket URL: <https://code.djangoproject.com/ticket/37097>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#37097: Nested double-unioned query for model with ordering fails on Postgres/MySql
-------------------------------------+-------------------------------------

Reporter: Shai Berger | Owner: Jacob
| Walls
Type: Bug | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Release blocker | 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 Jacob Walls):

* keywords: Regression =>
* owner: (none) => Jacob Walls
* stage: Unreviewed => Accepted
* status: new => assigned

Comment:

Thanks, I'll take a look today.
--
Ticket URL: <https://code.djangoproject.com/ticket/37097#comment:1>

[Django]

#37097: Nested double-unioned query for model with ordering fails on Postgres/MySql
-------------------------------------+-------------------------------------
Reporter: Shai Berger | Owner: Jacob
| Walls
Type: Bug | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Release blocker | 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 Jacob Walls):

* cc: Simon Charette (added)

Comment:

Bisected to 087bb9e8f3478d53f12b1737af865992af17c5f2, which seems like an
improvement.

SQL on stable/6.0.x:
{{{#!sql
SELECT ...

FROM "queries_author"
WHERE NOT ("queries_author"."id" IN ((

(SELECT U0."id" AS "id"
FROM "queries_author" U0
WHERE U0."num" > 7
ORDER BY U0."name" ASC)
UNION ALL
(SELECT U0."id" AS "id"
FROM "queries_author" U0
WHERE U0."num" < 2
ORDER BY U0."name" ASC))
UNION ALL
(SELECT U0."id" AS "id"
FROM "queries_author" U0
WHERE U0."num" = 5

ORDER BY U0."name" ASC)))

ORDER BY "queries_author"."name" ASC;
}}}
And on main:
{{{#!sql
SELECT ...

FROM "queries_author"
WHERE NOT ("queries_author"."id" IN ((

(SELECT "U0"."id" AS "id",

"U0"."name" AS
"__orderbycol2"
FROM "queries_author" "U0"
WHERE "U0"."num" > 7

ORDER BY "U0"."name" ASC)
UNION ALL
(SELECT "U0"."id" AS "id",

"U0"."name" AS
"__orderbycol2"
FROM "queries_author" "U0"
WHERE "U0"."num" < 2
ORDER BY "U0"."name" ASC)

ORDER BY "__orderbycol2" ASC)
UNION ALL

(SELECT "U0"."id" AS "id"

FROM "queries_author" "U0"
WHERE "U0"."num" = 5
ORDER BY "U0"."name" ASC)))

ORDER BY "queries_author"."name" ASC;
}}}
----
Two options come to mind:

**Option 1: take a lighter touch when setting default ordering:**
{{{#!diff
diff --git a/django/db/models/query.py b/django/db/models/query.py
index 6cdd7681b2..9d373410e2 100644
--- a/django/db/models/query.py
+++ b/django/db/models/query.py
@@ -1702,9 +1702,10 @@ class QuerySet(AltersData):
clone = self._chain()
# Clear limits and ordering so they can be reapplied
clone.query.clear_ordering(force=True)
- clone.query.default_ordering = True
clone.query.clear_limits()
clone.query.combined_queries = (self.query, *(qs.query for qs in
other_qs))
+ if not clone.query.combined_queries:
+ clone.query.default_ordering = True
clone.query.combinator = combinator
clone.query.combinator_all = all
return clone
}}}

**Option 2: Extend the fix from yesterday's Oracle/SQLite fixes to also
clear unnecessary orderings on Postgres:
**
{{{#!diff
diff --git a/django/db/models/sql/compiler.py
b/django/db/models/sql/compiler.py
index 764dc46cfc..bf90397ed8 100644
--- a/django/db/models/sql/compiler.py
+++ b/django/db/models/sql/compiler.py
@@ -636,13 +636,7 @@ class SQLCompiler:
if selected is not None and compiler.query.selected is None:
compiler.query = compiler.query.clone()
compiler.query.set_values(selected)
- if (
- (
- features.requires_compound_order_by_subquery
- and not
features.ignores_unnecessary_order_by_in_subqueries
- )
- or not features.supports_parentheses_in_compound
- ) and compiler.get_order_by():
+ if compiler.get_order_by():
compiler.query = compiler.query.clone()
compiler.query.clear_ordering(force=False)
part_sql, part_args = compiler.as_sql(with_col_aliases=True)
}}}

Simon, do you have an opinion about the best course of action here?
--
Ticket URL: <https://code.djangoproject.com/ticket/37097#comment:2>

[Django]

#37097: Nested double-unioned query for model with ordering fails on Postgres/MySql
-------------------------------------+-------------------------------------
Reporter: Shai Berger | Owner: Jacob
| Walls
Type: Bug | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Simon Charette):

When an `__in` lookup is used
[https://github.com/django/django/blob/335c6d0129400eda792f3bec5c71bb28af5e5d37/django/db/models/lookups.py#L508
we explicitly clear the ordering as it's unnecessary] (including the
default one).

This problem made me think of your initial approach at making
`Query.clear_ordering` [https://github.com/django/django/pull/21247/
somewhat recursive] Jacob. Should we instead consider making the following
changes instead?

{{{!diff
diff --git a/django/db/models/sql/query.py b/django/db/models/sql/query.py
index 45192b7809..603b7056ad 100644
--- a/django/db/models/sql/query.py
+++ b/django/db/models/sql/query.py
@@ -2402,6 +2402,8 @@ class Query(BaseExpression):
self.extra_order_by = ()
if clear_default:
self.default_ordering = False
+ for query in self.combined_queries:
+ query.clear_ordering(force=force,
clear_default=clear_default)

def set_group_by(self, allow_aliases=True):
"""
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/37097#comment:3>

[Django]

#37097: Nested double-unioned query for model with ordering fails on Postgres/MySql
-------------------------------------+-------------------------------------
Reporter: Shai Berger | Owner: Jacob
| Walls
Type: Bug | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Release blocker | 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 Jacob Walls):

* has_patch: 0 => 1

Comment:

And that was only yesterday!

Applied in [https://github.com/django/django/pull/21277 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/37097#comment:4>

[Django]

#37097: Nested double-unioned query for model with ordering fails on Postgres/MySql
-------------------------------------+-------------------------------------
Reporter: Shai Berger | Owner: Jacob
| Walls
Type: Bug | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Jacob Walls):

* needs_better_patch: 0 => 1

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

[Django]

#37097: Nested double-unioned query for model with ordering fails on Postgres/MySql
-------------------------------------+-------------------------------------
Reporter: Shai Berger | Owner: Jacob
| Walls
Type: Bug | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Release blocker | 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 Jacob Walls):

* needs_better_patch: 1 => 0

--
Ticket URL: <https://code.djangoproject.com/ticket/37097#comment:6>

[Django]

#37097: Nested double-unioned query for model with ordering fails on Postgres/MySql
-------------------------------------+-------------------------------------
Reporter: Shai Berger | Owner: Jacob
| Walls
Type: Bug | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Release blocker | Resolution:

Keywords: | Triage Stage: Ready for
| checkin

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Simon Charette):

* stage: Accepted => Ready for checkin

Comment:

Thanks for the great discussion and iteration Jacob!
--
Ticket URL: <https://code.djangoproject.com/ticket/37097#comment:7>

[Django]

#37097: Nested double-unioned query for model with ordering fails on Postgres/MySql
-------------------------------------+-------------------------------------
Reporter: Shai Berger | Owner: Jacob
| Walls

Type: Bug | Status: closed

Component: Database layer | Version: dev
(models, ORM) |

Severity: Release blocker | Resolution: fixed

Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Jacob Walls <jacobtylerwalls@…>):

In [changeset:"fbe902d4a4b8b9dcb371509b25ba8feff3852e64" fbe902d]:
{{{#!CommitTicketReference repository=""
revision="fbe902d4a4b8b9dcb371509b25ba8feff3852e64"
Refs #37097 -- Removed compilation-time order clearing on combined queries
on Oracle.

Thanks Simon Charette, JaeHyuck Sa, and Shai Berger for reviews.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/37097#comment:9>

[Django]

#37097: Nested double-unioned query for model with ordering fails on Postgres/MySql
-------------------------------------+-------------------------------------
Reporter: Shai Berger | Owner: Jacob
| Walls
Type: Bug | Status: closed
Component: Database layer | Version: dev
(models, ORM) |
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Jacob Walls <jacobtylerwalls@…>):

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

Comment:

In [changeset:"0e1d950fbb44878f6972a0e5730f69f98caf468e" 0e1d950]:
{{{#!CommitTicketReference repository=""
revision="0e1d950fbb44878f6972a0e5730f69f98caf468e"
Fixed #37097 -- Made Query.clear_ordering() clear ordering on combined
queries also.

Thanks Shai Berger for the report.

Regression in 087bb9e8f3478d53f12b1737af865992af17c5f2.

(That commit drove more traffic into an error that would have been
reachable only with an explicit order_by() after each union().)

Co-authored-by: Simon Charette <char...@gmail.com>
Co-authored-by: siddus <dcs...@gmail.com>
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/37097#comment:8>