[Django] #32673: ProgrammingError resulting from invalid SQL for nested models.Q instances
Django
-------------------------------------+-------------------------------------
Reporter: Charles | Owner: nobody
Lirsac |
Type: Bug | Status: new
Component: Database | Version: 3.0
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 |
-------------------------------------+-------------------------------------
Given the following model:
{{{
class Price(models.Model):
price = models.IntegerField(null=False)
price_previous = models.IntegerField(null=False)
on_sale = models.BooleanField(null=False)
}}}
The following query used to work in 2.2. but emits a `ProgrammingError` on
3.0+ against a Postgres database:
{{{
Price.objects.filter(
models.Q(
on_sale=models.ExpressionWrapper(
models.Q(price__lt=models.F('price_previous')),
output_field=models.BooleanField(),
)
)
)
}}}
Traceback:
{{{
Traceback (most recent call last):
File "<console>", line 5, in <module>
File ".../lib/python3.7/site-packages/django/db/models/query.py", line
256, in __repr__
data = list(self[:REPR_OUTPUT_SIZE + 1])
File ".../lib/python3.7/site-packages/django/db/models/query.py", line
280, in __iter__
self._fetch_all()
File ".../lib/python3.7/site-packages/django/db/models/query.py", line
1324, in _fetch_all
self._result_cache = list(self._iterable_class(self))
File ".../lib/python3.7/site-packages/django/db/models/query.py", line
51, in __iter__
results = compiler.execute_sql(chunked_fetch=self.chunked_fetch,
chunk_size=self.chunk_size)
File ".../lib/python3.7/site-packages/django/db/models/sql/compiler.py",
line 1169, in execute_sql
cursor.execute(sql, params)
File ".../lib/python3.7/site-packages/django/db/backends/utils.py", line
98, in execute
return super().execute(sql, params)
File ".../lib/python3.7/site-packages/django/db/backends/utils.py", line
66, in execute
return self._execute_with_wrappers(sql, params, many=False,
executor=self._execute)
File ".../lib/python3.7/site-packages/django/db/backends/utils.py", line
75, in _execute_with_wrappers
return executor(sql, params, many, context)
File ".../lib/python3.7/site-packages/django/db/backends/utils.py", line
84, in _execute
return self.cursor.execute(sql, params)
File ".../lib/python3.7/site-packages/django/db/utils.py", line 90, in
__exit__
raise dj_exc_value.with_traceback(traceback) from exc_value
File ".../lib/python3.7/site-packages/django/db/backends/utils.py", line
84, in _execute
return self.cursor.execute(sql, params)
django.db.utils.ProgrammingError: syntax error at or near "<"
LINE 1: ..."prices_price"."on_sale" = "prices_price"."price" < "prices_...
}}}
The error comes from the way the query ended up compiled, in 3.2 we get:
{{{
SELECT "prices_price"."id", "prices_price"."price",
"prices_price"."price_previous", "prices_price"."on_sale"
FROM "prices_price"
WHERE "prices_price"."on_sale" = "prices_price"."price" <
"prices_price"."price_previous"
}}}
The right hand side of the clause is not wrapped in parentheses. While on
2.2 we get:
{{{
SELECT "prices_price"."id", "prices_price"."price",
"prices_price"."price_previous", "prices_price"."on_sale"
FROM "prices_price"
WHERE "prices_price"."on_sale" = ("prices_price"."price" <
("prices_price"."price_previous"))
}}}
The right hand side is correctly wrapped.
Sorry if the title is not super accurate, I wasn't sure how to exactly to
describe the exact kind of models.Q invocation at play here.
Versions:
- Python 3.7
- Tested agains postgres 9.6, 11 and 13
- Tested against 3.0.14, 3.1.8 and 3.2. It works on 2.2.20
For reference. the following query (valid in 3.0+) is equivalent and
generates the same broken SQL:
{{{
Price.objects.filter(
models.Q(
on_sale=models.Q(price__lt=models.F('price_previous'))
)
)
}}}
I've logged more details and a complete reproduction at
https://github.com/lirsacc/django-check-constraint-pg-regression (in the
context of a `CheckConstraint` and migrations which is where we first saw
this).
--
Ticket URL: <https://code.djangoproject.com/ticket/32673>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 3.0
(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
-------------------------------------+-------------------------------------
Old description:
New description:
Given the following model:
Traceback:
The right hand side is correctly wrapped (even if some parts are over
wrapped).
Sorry if the title is not super accurate, I wasn't sure how to exactly to
describe the exact kind of models.Q invocation at play here.
Versions:
- Python 3.7
- Tested agains postgres 9.6, 11 and 13
- Tested against 3.0.14, 3.1.8 and 3.2. It works on 2.2.20
For reference. the following query (valid in 3.0+) is equivalent and
generates the same broken SQL:
{{{
Price.objects.filter(
models.Q(
on_sale=models.Q(price__lt=models.F('price_previous'))
)
)
}}}
I've logged more details and a complete reproduction at
https://github.com/lirsacc/django-check-constraint-pg-regression (in the
context of a `CheckConstraint` and migrations which is where we first saw
this).
--
--
Ticket URL: <https://code.djangoproject.com/ticket/32673#comment:1>
Django
-------------------------------------+-------------------------------------
Reporter: Charles Lirsac | Owner: nobody
Component: Database layer | Version: 3.0
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: Simon Charette (added)
* keywords: => postgresql oracle q
* stage: Unreviewed => Accepted
Comment:
Thanks for the report.
Regression in 3a505c70e7b228bf1212c067a8f38271ca86ce09.
Reproduced at 6d0cbe42c3d382e5393d4af48185c546bb0ada1f.
--
Ticket URL: <https://code.djangoproject.com/ticket/32673#comment:2>
Django
-------------------------------------+-------------------------------------
Reporter: Charles Lirsac | Owner: nobody
Component: Database layer | Version: 3.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: postgresql oracle q | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/32673#comment:3>
Django
-------------------------------------+-------------------------------------
| Charette
Type: Bug | Status: assigned
Component: Database layer | Version: 3.0
(models, ORM) |
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: nobody => Simon Charette
* status: new => assigned
* stage: Accepted => Ready for checkin
Comment:
[https://github.com/django/django/pull/14302 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/32673#comment:4>
Django
-------------------------------------+-------------------------------------
Reporter: Charles Lirsac | Owner: Simon
| Charette
Component: Database layer | Version: 3.0
(models, ORM) |
Keywords: postgresql oracle q | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"170b006ce82b0ecf26dc088f832538b747ca0115" 170b006c]:
{{{
#!CommitTicketReference repository=""
revision="170b006ce82b0ecf26dc088f832538b747ca0115"
Fixed #32673 -- Fixed lookups crash when comparing against lookups on
PostgreSQL.
Regression in 3a505c70e7b228bf1212c067a8f38271ca86ce09.
Nonlitteral right-hand-sides of lookups need to be wrapped in
parentheses to avoid operator precedence ambiguities.
Thanks Charles Lirsac for the detailed report.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32673#comment:5>
Django
-------------------------------------+-------------------------------------
Reporter: Charles Lirsac | Owner: Simon
| Charette
Type: Bug | Status: closed
Component: Database layer | Version: 3.0
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: postgresql oracle q | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by GitHub <noreply@…>):
In [changeset:"0aacbdcf27b258387643b033352e99e6103abda8" 0aacbdcf]:
{{{
#!CommitTicketReference repository=""
revision="0aacbdcf27b258387643b033352e99e6103abda8"
Refs #32673 -- Fixed lookups crash when comparing against lookups on
Oracle.
Follow up to 170b006ce82b0ecf26dc088f832538b747ca0115.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32673#comment:6>
Django
-------------------------------------+-------------------------------------
Reporter: Charles Lirsac | Owner: Simon
| Charette
Type: Bug | Status: closed
Component: Database layer | Version: 3.0
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: postgresql oracle q | Triage Stage: Ready for
| checkin
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
{{{#!CommitTicketReference repository=""
revision="33c06ca0da6c4f151b84e5d8c305faff9ca30d98"
Refs #32673, Refs #35295 -- Avoided wrapping rhs direct values in lookups.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32673#comment:7>