Opened 19 years ago
Last modified 6 hours ago
#6343 assigned Bug
% symbols not escaped in db_column column names when preparing queries
| Reported by: | Owned by: | Garvit Sharma | |
|---|---|---|---|
| Component: | Database layer (models, ORM) | Version: | dev |
| Severity: | Normal | Keywords: | |
| Cc: | Annabelle Wiegart | Triage Stage: | Accepted |
| Has patch: | yes | Needs documentation: | yes |
| Needs tests: | yes | Patch needs improvement: | yes |
| Easy pickings: | no | UI/UX: | no |
Description
Using % in database column names (specified using db_column) causes the database wrapper to fail when preparing queries.
This is because the % symbol is not properly quoted (as %%), and conflicts with the usage of %s for passing parameters to queries.
I am attaching a patch for the MySQL backend where I encountered the issue; I'm not sure if other backends exhibit this bug because it presumably depends both on whether the database's native capability to support % characters in column names, and on the Python DB-API paramstyle.
Attachments (1)
Change History (14)
by , 19 years ago
| Attachment: | percent_column_names.diff added |
|---|
comment:1 by , 19 years ago
| Triage Stage: | Unreviewed → Design decision needed |
|---|
You can use % in db column names? I really think that's a bad bad idea to start with, but I'll put this as Design Dec. Needed just in case.
comment:2 by , 16 years ago
| Severity: | → Normal |
|---|---|
| Type: | → Bug |
Guessing from the report, I take it this is a bug, but I'm not sure if it is still a valid one though...
comment:3 by , 16 years ago
| Needs tests: | set |
|---|
comment:5 by , 16 years ago
comment:6 by , 15 years ago
| Easy pickings: | unset |
|---|---|
| Triage Stage: | Design decision needed → Accepted |
| UI/UX: | unset |
This either needs to be fixed, or if that's not technically feasible in a sane way, documented.
#11391 closed as duplicate of this, also raises the issue that this can happen with field names, not just table names. That should also be addressed.
comment:7 by , 5 months ago
| Owner: | changed from to |
|---|---|
| Status: | new → assigned |
comment:8 by , 5 months ago
| Owner: | removed |
|---|---|
| Status: | assigned → new |
follow-up: 12 comment:9 by , 4 weeks ago
| Owner: | set to |
|---|---|
| Status: | new → assigned |
Still reproducible on main (08e4c0d8e7), though the failure is not where the original patch targets.
Minimal reproduction:
class Thing(models.Model): price = models.IntegerField(db_column="cost%")
str(Thing.objects.filter(price=5).query) ValueError: unsupported format character '"' (0x22) at index 55 str(Thing.objects.all().query) TypeError: not enough arguments for format string
Both come from Query.str in django/db/models/sql/query.py:
def str(self): sql, params = self.sql_with_params() return sql % params
The % in the column name is read as the start of a format specifier by the % operator. With params present it fails on the character following %; with no params it fails for lack of arguments.
sql_with_params() is unaffected and returns the correct SQL and parameters:
Thing.objects.filter(price=5).query.sql_with_params() ('SELECT "check_app_thing"."id", "check_app_thing"."cost%" FROM "check_app_thing" WHERE "check_app_thing"."cost%" = %s', (5,))
Query execution also works on SQLite, where the driver uses qmark paramstyle. I have not tested execution on MySQL or PostgreSQL, where substitution happens inside the driver — that path may still fail as originally reported.
Since Query.str is in the generic SQL layer rather than a backend, this failure appears to be backend-independent, but I have only tested SQLite.
I'd like to work on this.
comment:10 by , 12 days ago
Takes the narrow approach: Query.__str__() returns the uninterpolated SQL
rather than raising when an identifier contains %.
I also tried the fuller fix - escaping % in quote_name() like Oracle
does. This corrupts DDL: CREATE TABLE carries the escaped name through
and the column ends up called cost%%, which should have been cost%.
SQLite already has a %% → % collapse, and making that collapse
unconditional fixes SQLite, but MySQL and PostgreSQL have no collapse step
at all and one would have to be added. That would mean % processing on
paths that currently do none.
Happy to go that way instead if preferred.
comment:11 by , 11 days ago
| Needs documentation: | set |
|---|---|
| Patch needs improvement: | set |
comment:12 by , 5 days ago
Replying to Garvit Sharma:
Still reproducible on main (08e4c0d8e7), though the failure is not where the original patch targets.
Minimal reproduction:
class Thing(models.Model): price = models.IntegerField(db_column="cost%")str(Thing.objects.filter(price=5).query) ValueError: unsupported format character '"' (0x22) at index 55 str(Thing.objects.all().query) TypeError: not enough arguments for format stringBoth come from Query.str in django/db/models/sql/query.py:
def str(self): sql, params = self.sql_with_params() return sql % paramsThe % in the column name is read as the start of a format specifier by the % operator. With params present it fails on the character following %; with no params it fails for lack of arguments.
sql_with_params() is unaffected and returns the correct SQL and parameters:
Thing.objects.filter(price=5).query.sql_with_params() ('SELECT "check_app_thing"."id", "check_app_thing"."cost%" FROM "check_app_thing" WHERE "check_app_thing"."cost%" = %s', (5,))Query execution also works on SQLite, where the driver uses qmark paramstyle. I have not tested execution on MySQL or PostgreSQL, where substitution happens inside the driver — that path may still fail as originally reported.
Since Query.str is in the generic SQL layer rather than a backend, this failure appears to be backend-independent, but I have only tested SQLite.
I'd like to work on this.
Hey Garvit, thank you for giving this a try! :)
I was unable to reproduce your query in DryORM. The ValueError already occurs at table creation. I'm wondering whether we should provide an error message stating that "%" is forbidden in column names in that case. That doesn't solve the problem with queries in legacy projects with database columns containing "%" though.
Would you be able to share how reproduce your query?
comment:13 by , 7 hours ago
| Cc: | added |
|---|
Patch for MySQL backend