Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#24015 closed Bug (fixed)

Django doesn't quote index names in SQL generated by migrations.

Reported by: maraneta Owned by: nobody
Component: Migrations Version: 1.7
Severity: Release blocker Keywords: migrations, testing, sql
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

Django does NOT quote index names in SQL generated by migrations.

Using a table name with a space in it (by specifying it with 'db_table' in a model's Meta options) will result in an error when trying to migrate. This error only occurs if the model has a foreign key field, because only then will the resulting SQL contain a 'create index' query for that model.

This is very easy to replicate; I'll slightly change the django tutorial models as an example. I've created a new project, with an app called 'Polls'.

models.py:

from django.db import models

class Question(models.Model):
    question_text = models.CharField(max_length=200)
    pub_date = models.DateTimeField('date published')
    
class Choice(models.Model):
    question = models.ForeignKey(Question)
    choice_text = models.CharField(max_length=200)
    votes = models.IntegerField(default=0)
        
    class Meta:
        db_table = u'Choice Table'

Once you have these models and a clean database:

  1. Run python manage.py migrate to generate the initial tables if you haven't already.
  2. Run python manage.py makemigrations polls to create the initial migration file for the polls app.

Note: If you run python manage.py sqlmigrate polls 0001 right now, you can see the following SQL that is supposed to be executed with this migration:

BEGIN;
CREATE TABLE "Choice Table" ("id" serial NOT NULL PRIMARY KEY, "choice_text" varchar(200) NOT NULL, "votes" integer NOT NULL);
CREATE TABLE "polls_question" ("id" serial NOT NULL PRIMARY KEY, "question_text" varchar(200) NOT NULL, "pub_date" timestamp with time zone NOT NULL);
ALTER TABLE "Choice Table" ADD COLUMN "question_id" integer NOT NULL;
ALTER TABLE "Choice Table" ALTER COLUMN "question_id" DROP DEFAULT;
CREATE INDEX Choice Table_7aa0f6ee ON "Choice Table" ("question_id");
ALTER TABLE "Choice Table" ADD CONSTRAINT "Choice Table_question_id_3209f4dfb997962d_fk_polls_question_id" FOREIGN KEY ("question_id") REFERENCES "polls_question" ("id") DEFERRABLE INITIALLY DEFERRED;

COMMIT;
  1. If you did step 1 before you created the models, you can just run python manage.py migrate polls to get the error. If you did step 1 after you created the models, you won't get an error in migrating because the 'Choice' and 'Question' relations will already exist and the migration will be 'faked'. However, you can get the error either way by running python manage.py test polls , as this will run these migrations on a clean temporary test database.

This is the output when trying to run tests:

...
django.db.utils.ProgrammingError: syntax error at or near "Table_7aa0f6ee"
LINE 1: CREATE INDEX Choice Table_7aa0f6ee ON "Choice Table" ("quest...

Looking at this error and the SQL code output by sqlmigrate , this error is a result of django not quoting "Choice Table_7aa0f6ee" when it's creating an index.

Keep in mind that this error will only occur if the index (from the foreign key field) is created at the same time or after the table is named with a space in it.
If you exclude the meta options above and migrate, then add it in a later migration, there will be no error. However, it is difficult to use this workaround when I already have an existing database.

Quoting these SQL index names will allow users to use spaces in their db_table names without error.

Change History (6)

comment:1 Changed 6 years ago by Tim Graham

Severity: NormalRelease blocker
Triage Stage: UnreviewedAccepted

Tracked it down to a missing quote_name() here. It looks like this was fixed in master by 6072f17d09cbe8b7cecfbfa1a253d0a4f2152170. A good fix is probably to backport that commit to 1.7.x and add a test along with it. The committer can then forwardport the test.

comment:3 Changed 6 years ago by Tim Graham

Triage Stage: AcceptedReady for checkin

comment:4 Changed 6 years ago by Claude Paroz <claude@…>

Resolution: fixed
Status: newclosed

In f46a16614d0a8339362ed7a48eef8f1914f96c85:

[1.7.x] Fixed #24015 -- Factorized create_index_sql expression

Backport of 6072f17d0 from master, with one test reinforced.
Thanks Tim Graham for the review.

comment:5 Changed 6 years ago by Claude Paroz <claude@…>

In 5b1fb0a75d6961d78ba68c72008f1cc535f9689a:

Forward-ported test and release note from f46a16614

Refs #24015.

comment:6 Changed 6 years ago by Claude Paroz

@maraneta: Sorry, I just realize now that I didn't credited you in the commit message :-( Thanks anyway!

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