#29891 closed Cleanup/optimization (duplicate)
Fix incorrect quoting in queryset.query
| Reported by: | Timothy Allen | Owned by: | Timothy Allen |
|---|---|---|---|
| Component: | Database layer (models, ORM) | Version: | 2.1 |
| Severity: | Normal | Keywords: | queryset, orm |
| Cc: | Triage Stage: | Accepted | |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | yes | Patch needs improvement: | yes |
| Easy pickings: | no | UI/UX: | no |
Description
This old ticket has some recent activity I have also run into with fields not being properly quoted in queryset.query:
https://code.djangoproject.com/ticket/14091
For a minimum example, start an app and create a model with a DateField:
from django.db import models
class BadDateModel(models.Model):
my_date = models.DateField()
Then fire up a shell:
>>> q = BadDateModel.objects.filter(my_date__gt='1990-01-01') >>> print(q.query) SELECT "bug_app_baddatemodel"."id", "bug_app_baddatemodel"."my_date" FROM "bug_app_baddatemodel" WHERE "bug_app_baddatemodel"."my_date" > 1990-01-01
As you can see, the generated query has a date value which is not quoted, which will cause an error when executed. This is causing me issues when trying to do an EXECUTE {queryset.query} to get an estimate of table size before performing certain DRF queries.
The error when it is executed against PostgreSQL:
operator does not exist: date < integer
LINE 1: ... FROM "blah"."blah1" WHERE ("blah"."blah1"."date" < 1990-01-...
^
HINT: No operator matches the given name and argument type(s). You might need to add explicit type casts.
I discovered this in PostgreSQL, but it seems to affect all backends; the repro above was done with SQLite.
Change History (8)
comment:1 by , 7 years ago
comment:2 by , 7 years ago
I'll take a look and see if I can implement the same logic Aymeric implemented for last_executed_query. Thanks for pointing me to the ticket!
comment:3 by , 7 years ago
| Owner: | changed from to |
|---|---|
| Status: | new → assigned |
comment:4 by , 7 years ago
| Triage Stage: | Unreviewed → Accepted |
|---|---|
| Type: | Bug → Cleanup/optimization |
I setup SQL logging via LOGGING dictConfig and verified that as suggested, this only affects print(queryset.query). Therefore, I'd describe this as cleanup/optimization rather than a bug.
comment:5 by , 7 years ago
I've put in a work-in-progress PR: https://github.com/django/django/pull/10568
This is my first time diving into any part of the ORM source, so I'd love some feedback / suggestions. It currently causes a few tests to fail, I believe from the removal of the ESCAPE.
comment:6 by , 7 years ago
This is related to #27587, which concerns documenting Query.query.
Aymeric's comment there is worth having here:
Regarding the limitations, I don't think we should accept the current status quo as the best we can do:
- if it's reasonably easy to perform the escaping correctly, typically just by escaping arguments with a function provided by the database adapter prior to interpolation, then Django should do it.
- if there's no function for escaping arguments, but a well documented and not too complicated process do to so (replace " by "" then wrap in "...") then we should consider doing it as well
- if the escaping rules are unclear and there's no way to ask the database to do it, then we should return something that is *obviously* invalid, like QUERY = ... ; PARAMS = ...
I did that some time ago for last_executed_query. SQLite stayed at 3 for half a decade before moving to 2.
I believe the same logic should apply to QuerySet.query and similar methods.
comment:7 by , 7 years ago
| Has patch: | set |
|---|---|
| Needs tests: | set |
| Patch needs improvement: | set |
comment:8 by , 6 years ago
| Resolution: | → duplicate |
|---|---|
| Status: | assigned → closed |
Closing this as a duplicate of #25705.
QuerySet.__str__()isn't meant to give executable SQL, see #27587. That said, Aymeric's comment there suggests to improve the situation if possible. Do you want to try a patch?