Modify

Ticket #572 (closed defect: fixed)

Opened 4 years ago

Last modified 4 years ago

Incompatibility with tables manually specifying postgres schemas

Reported by: agrossman@… Owned by: andrew
Priority: major Milestone: 0.7.3
Component: commands Version: unknown
Keywords: postgres schema Cc:

Description

A method for specifying postgres schemas to be used within Django is specified here : http://www.mail-archive.com/django-users@googlegroups.com/msg54866.html .

From this, the syntax for specifying a schema-located table when declaring a Django model is:

    class Meta:
        db_table = '"myschema"."mytable"'

South is compatible with the creation of tables; however, when trying to create a table that references another table using this method, South migrate encounters an error.

The SQL it generates is in the following format:

CREATE INDEX ""myschema"."mytable"_my_foreign_column_id" ON "myschema"."mytable" ("my_foreign_column_id");

The quoting mechanism is not currently fully compatible with this syntax; while it could be considered non-standard Django, it would seem that correcting this problem would not be overly difficult.

The fix in south/db/generic.py is as follows:

index_name = ('%s_%s%s%s' % (table_name, column_names[0], index_unique_name, suffix))

becomes

index_name = ('%s_%s%s%s' % (table_name, column_names[0], index_unique_name, suffix)).replace('"', '').replace('.', '_')

I have attached a patch that fixes the problem. It is applied to south/db/generic.py .

Attachments

generic.py.patch (1.2 KB) - added by anonymous 4 years ago.
generic.py.2.patch (1.5 KB) - added by agrossman@… 4 years ago.
An updated patch that addresses more problems encountered

Change History

Changed 4 years ago by anonymous

Changed 4 years ago by agrossman@…

An updated patch that addresses more problems encountered

comment:1 Changed 4 years ago by anonymous

Additionally, the line

table_name = table_name.replace('"', '').replace('.', '_')

should be added at the beginning of the create_index_name function. I have attached an updated patch.

comment:2 Changed 4 years ago by andrew

  • Status changed from new to assigned
  • Milestone set to 1.0

comment:3 Changed 4 years ago by andrew

  • Status changed from assigned to closed
  • Resolution set to fixed
  • Milestone changed from 1.0 to 0.7.3

Committed in [e7599dd243bf].

comment:4 Changed 4 years ago by googletorp

  • Status changed from closed to reopened
  • Resolution fixed deleted

This only partly fixes this issue, which is related to django.

Russ states in http://code.djangoproject.com/ticket/14203 that the "technique" is by accident and not be design, which means that django doesn't support this. This is seen, when you in django try to create an index, you run into the exact same error as you've fixed in South.

The problem with South is that upon creating a new table you do

if hasattr(self._get_connection().creation, "sql_indexes_for_field"):
    # Make a fake model to pass in, with only db_table
    model = self.mock_model("FakeModelForGISCreation", table_name)
    for stmt in self._get_connection().creation.sql_indexes_for_field(model, field, no_style()):
        self.add_deferred_sql(stmt)

Here you rely on django's sql_indexes_for_field which did exactly what South did before producing SQL that looks like this:

CREATE INDEX ""myschema"."mytable"_my_foreign_column_id"
ON "myschema"."mytable" ("my_foreign_column_id");

So this brings up as I see it 3 choices:

  • Don't support like django
  • Don't rely on django for SQL
  • Fix the SQL flaw that django produces in sql_indexes_for_field

Eventhough I would like to see schema support for PostgresSQL, it might not make so much sense when django doesn't actually support it (or only support it in some ways though hacks and accidents)

Since keeping data integrity is vital for a project like this, it might make more sense not to support this feature until Django itself comes up with a way of dealing with this.

Anyways I'll let you guys figure out what should happen, so I'm reopening this issue.

comment:5 Changed 4 years ago by andrew

  • Status changed from reopened to closed
  • Resolution set to fixed

To be honest, I don't want to invest any more time in fixing this issue - the original fix was quick and dirty, but as you say, Django doesn't officially support it, and until they do, and have a good way of getting the schema, I'm going to re-close this as fixed.

If you can up with a patch to fix the sql_indexes_for_field problem, I might well apply it, though.

View

Add a comment

Modify Ticket

Action
as closed
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.