Opened 7 years ago

Closed 7 years ago

#24585 closed Cleanup/optimization (wontfix)

Remove repetitive guarding against double quoting database identifiers

Reported by: Jon Dufresne Owned by: nobody
Component: Database layer (models, ORM) Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

All built-in database backends define a quote_name() function. In all cases, the function first checks if the string argument is already quoted.

IMO, this looks a bit like "just in case" style programming and a bit of a code smell. Ideally, the calling code should not be passing quoted names in the first place. The Django logic should be such that names are quoted exactly once and not multiple time. I believe if names are quoted twice, it should be treated as a bug in the calling code, not a bug in the quote_name() function.

After testing, these guards are unnecessary and all tests pass without it across multiple database backends.

Removing the guard means it won't need to be checked every time SQL names are quoted.

PR to follow.

Change History (4)

comment:2 Changed 7 years ago by Claude Paroz

I know about one use case (undocumented/untested) where this check might be useful. It's about unmanaged PostgreSQL tables in non-default (i.e. not public) schema (the target of #6148). Currently, you can define the db_table meta attribute to '"schema"."table_name"', and it works, at least with standard ORM methods. I'm afraid that your patch will break this.

comment:3 Changed 7 years ago by Aymeric Augustin

Does the possibility of backwards-incompatibility exist if user code relies on the current behavior?

If I have to choose between "make things look a bit more clean" and "avoid breaking user code" and there are no other advantages, the latter wins.

comment:4 Changed 7 years ago by Shai Berger

Resolution: wontfix
Status: newclosed

This is also used by Oracle users to reference out-of-schema tables and to preserve case (normally, the Oracle backend turns all names to uppercase). We can think up cleaner alternatives, but the current behavior cannot be simply removed without replacement; and this replacement would need to go through a deprecation cycle as well.

The right place for discussing such alternatives is the DevelopersMailingList.

Note: See TracTickets for help on using tickets.
Back to Top