Opened 4 years ago
Closed 4 years ago
#33435 closed Cleanup/optimization (fixed)
Subquery.as_sql() generates invalid SQL.
| Reported by: | M1ha Shvn | Owned by: | Mohamed Nabil Rady |
|---|---|---|---|
| Component: | Database layer (models, ORM) | Version: | dev |
| Severity: | Normal | Keywords: | subquery, as_sql |
| Cc: | Simon Charette | Triage Stage: | Ready for checkin |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | yes | UI/UX: | no |
Description (last modified by )
Since this commit Subquery.as_sql(...) method returns incorrect SQL removing first and last symbols instead of absent breakets. Adding Subquery().query.subquery = True attribute fixes the problem. From my point of view, it should be set in Subquery constructor.
from django.db import connection
from apps.models import App
q = Subquery(App.objects.all())
print(str(q.query))
# Output SQL is valid:
# 'SELECT "apps_app"."id", "apps_app"."name" FROM "apps_app"'
print(q.as_sql(q.query.get_compiler('default'), connection))
# Outptut SQL is invalid (no S letter at the beggining and " symbol at the end):
# ('(ELECT "apps_app"."id", "apps_app"."name" FROM "apps_app)', ())
q.query.subquery = True
print(q.as_sql(q.query.get_compiler('default'), connection))
# Outputs correct result
('(SELECT "apps_app"."id", "apps_app"."name" FROM "apps_app")', ())
Change History (7)
comment:1 by , 4 years ago
| Description: | modified (diff) |
|---|
comment:2 by , 4 years ago
| Cc: | added |
|---|---|
| Summary: | Since django 3.0 Subquery.as_sql() generates invalid SQL → Subquery.as_sql() generates invalid SQL. |
| Triage Stage: | Unreviewed → Accepted |
| Type: | Bug → Cleanup/optimization |
comment:3 by , 4 years ago
Sounds reasonable to me as well, I'd only suggest we .clone() the query before altering though.
comment:4 by , 4 years ago
| Easy pickings: | set |
|---|---|
| Has patch: | set |
| Owner: | changed from to |
| Status: | new → assigned |
Pull request: https://github.com/django/django/pull/15320.
Ran back-end tests and they went fine.
comment:5 by , 4 years ago
| Needs tests: | set |
|---|
comment:6 by , 4 years ago
| Needs tests: | unset |
|---|---|
| Triage Stage: | Accepted → Ready for checkin |
Note:
See TracTickets
for help on using tickets.
Sounds reasonable.