% symbols not escaped in db_column column names when preparing queries

[Daniel Pope <dan@…>]

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.

[Daniel Pope <dan@…>]

Patch for MySQL backend

[Simon Greenhill <dev@…>]

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.

[Julien Phalip]

Guessing from the report, I take it this is a bug, but I'm not sure if it is still a valid one though...

[Julien Phalip]

See #11391 for a potentially related issue.

[Julien Phalip]

See some other issues relating to the use of the % character and escaping: #3485, #12268, #11391, #13648. Perhaps some of these could be tackled at the same time.

[Carl Meyer]

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.

[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 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.

[Garvit Sharma]

​PR

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.

[Annabelle Wiegart]

Hey Garvit, thank you for giving this a try! :)

As you can see in ​DryORM, the ValueError also occurs at instance creation. I'm wondering whether we should provide an error message stating that "%" is forbidden in column names in that case. Wouldn't it be better not to allow that character to begin with?

That doesn't solve the problem of making queries in legacy projects with database columns containing "%" though. I'm not sure that's a real need, but from reading this ​Google groups comment linked in #11391, it could be with legacy projects.