Opened 3 weeks ago

Last modified 13 days ago

#37365 assigned Bug

models.E007 misses duplicate columns when one db_column is explicitly quoted

Reported by: Matthew Schinckel Owned by: Md. Saikat Islam
Component: Core (System checks) Version: 6.1
Severity: Normal Keywords: db_column quoting system-checks
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

The models.E007 system check can miss duplicate database columns when one db_column is explicitly quoted and another is not. The strings differ, but the configured backend can render them as the same SQL identifier.

This issue appears to be mostly the same across different backends.

Reproduction

This test uses the configured backend's own identifier quoting. It does not query the database.

from django.db import connection, models
from django.test import SimpleTestCase
from django.test.utils import isolate_apps


class QuotedColumnTests(SimpleTestCase):
    @isolate_apps()
    def test_quoted_column_collision(self) -> None:
        quoted_name = connection.ops.quote_name("id")

        class Example(models.Model):
            key = models.IntegerField(primary_key=True, db_column=quoted_name)
            score = models.IntegerField(db_column="id")

            class Meta:
                app_label = "example"

        self.assertIn("models.E007", {error.id for error in Example.check()})

Actual result: Example.check() returns [], so the assertion fails.

Expected result: models.E007, because both fields map to the same physical column. Using the unquoted string "id" for both fields correctly produces that error.

Backend examples

The equivalent spellings depend on the backend:

  • SQLite: id and "id" both render as "id". ​Source
  • MySQL: id and `id` both render as `id`. The quoted spelling must use backticks; double quotes would represent a different identifier. ​Source
  • Oracle: id and "id" both render as "ID", because Django's Oracle quote_name() uppercases its result. ​Source
  • PostgreSQL: id and "id" both render as "id". ​Source

Cause

​Model._check_column_name_clashes() compares the original field.column strings without accounting for backend identifier handling. It accepts declarations that would generate duplicate column names.

The desired behaviour is to recognise equivalent identifiers and report the collision, rather than permit duplicate columns. Any fix should account for backend-specific rules rather than simply stripping double quotes.

Versions checked

Runtime reproduction: Django 5.2.17, Python 3.11.6, with SQLite and PostgreSQL backend configurations. The checks do not query a database.

6.1.1 appears to have the same issue based on inspection of its released source. The shared check and the four backends' quote_name() implementations are unchanged from 5.2.17. MySQL and Oracle findings are source verification only; no integration tests were run against those databases.

Change History (8)

comment:1 by Md. Saikat Islam, 3 weeks ago

Owner: set to Md. Saikat Islam
Status: new → assigned
Triage Stage: Unreviewed → Accepted

Thanks for reporting this issue. I was able to reproduce it after inspection.

After digging into the code I found the reason. When we normally assign a string value as db_column, django saves it as it is ("id"), but when we use connections.quoate_name djangos makes it quoated '"id"'. As a result in the _check_column_name_clashes (django/db/models/base.py) method when we compare using

...
     column_name = f.column

     # Ensure the column name is not already in use.
      if column_name and column_name in used_column_names:
         errors.append(...)
....

This could be solved using column_name = connection.ops.quote_name(f.column) so that we can treat "id" and '"id"' as same in duplicacy check.

but this is not good when querying data using sql if we assign a db_column name like "multi word column name". This should be quoated.
So another solution i found is to update the field db_column name in init as quated.
in django/db/models/fields/__init__.py

self.db_column = db_column if not db_column else connection.ops.quote_name(db_column)

since quote_name follows quoting once rule. So this should be good to go. I might be following first solution.

Last edited 3 weeks ago by Md. Saikat Islam (previous) (diff)

comment:2 by Md. Saikat Islam, 3 weeks ago

On a second thought.

db_column = "id" ---------------------------------------------------> gives you database column name id
db_column = connection.ops.quote_name("id") ---> gives you database column name "id"

So they are not duplicate and should not raise duplicate column name error. But we have duplicate column name error in database level when we try to run migration.
Also django doc specifies:

... Django quotes column and table names behind the scenes.

So they are quoted before being executed.

Last edited 3 weeks ago by Md. Saikat Islam (previous) (diff)

comment:3 by Matthew Schinckel, 3 weeks ago

Yeah, I was going to work through a solution but ran out of day.

in reply to:  3 comment:4 by Md. Saikat Islam, 3 weeks ago

I am already done with the solution. I was going to open a PR. I’d appreciate it if you could share your thoughts on my reasoning.

comment:5 by Md. Saikat Islam, 3 weeks ago

Has patch: set

comment:7 by Jacob Walls, 2 weeks ago

A note for you Saikat, about triaging. Often new reporters aren't familiar with the owner field. It's better to ask them if they want to submit a PR and leave the owner field blank until they respond with a No or silence. Otherwise assigning to yourself might appear as poaching.

comment:8 by Md. Saikat Islam, 13 days ago

Got it, thanks for pointing that out. I'll make sure to follow this process moving forward.

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