[Django] #28646: Migration calls "CREATE INDEX" when one already exists when 'unique' field attribute is added

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
11 messages Options
Reply | Threaded
Open this post in threaded view
|

[Django] #28646: Migration calls "CREATE INDEX" when one already exists when 'unique' field attribute is added

Django
#28646: Migration calls "CREATE INDEX" when one already exists when 'unique' field
attribute is added
-------------------------------------+-------------------------------------
               Reporter:  Hari - 何  |          Owner:  nobody
  瑞理                               |
                   Type:  Bug        |         Status:  new
              Component:  Database   |        Version:  1.11
  layer (models, ORM)                |       Keywords:
               Severity:  Normal     |  postgresql,migration,index
           Triage Stage:             |      Has patch:  0
  Unreviewed                         |
    Needs documentation:  0          |    Needs tests:  0
Patch needs improvement:  0          |  Easy pickings:  0
                  UI/UX:  0          |
-------------------------------------+-------------------------------------
 PostgreSQL migration automatically creates an index for fields that set
 db_index=True. An example is SlugField, which sets this property
 implicitly. Thereafter when the unique=True property is added to the field
 the resultant migration script generates an AlterField object to apply
 this unique attribute. The schema editor then incorrectly detects this new
 unique=True attribute to indicate the need to create a like index
 statement on the field which causes an error as it conflicts with the
 already existing index.

 The offending piece of code seems to be at
 django/db/backends/postgresql/schema.py:117.

 {{{
  if ((not (old_field.db_index or old_field.unique) and new_field.db_index)
 or
                 (not old_field.unique and new_field.unique)):
             like_index_statement = self._create_like_index_sql(model,
 new_field)
             if like_index_statement is not None:
                 self.execute(like_index_statement)
 }}}

 If it's changed as:
 {{{
  if (not (old_field.db_index or old_field.unique) and (new_field.db_index
 or new_field.unique)):
             like_index_statement = self._create_like_index_sql(model,
 new_field)
             if like_index_statement is not None:
                 self.execute(like_index_statement)
 }}}

 this error won't occur.

 PostgreSQL 9.5 introduces 'IF NOT EXISTS' to the CREATE INDEX statement
 which if added to the schema template can also address this problem
 without changing the above logic.

 I encountered the problem with SlugField() which implicitly sets db_index
 = True on PostgreSQL 9.4.

 Interestingly, I only discovered this when I used django-tenant-schemas
 which adds a thin layer on top of the default Database router setting the
 schema search path before handing over the work to the default router.
 With a vanilla Django installation using default router, the second call
 to create a like index does not throw an error. However, upon reviewing
 the code, the logic does look incorrect.

 I'm still investigating to see if this there's more to this than what I
 just described.

--
Ticket URL: <https://code.djangoproject.com/ticket/28646>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/051.9649bea132b84e68e68e44fd29ad0ba9%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #28646: Migration calls "CREATE INDEX" when one already exists when 'unique' field attribute is added

Django
#28646: Migration calls "CREATE INDEX" when one already exists when 'unique' field
attribute is added
-------------------------------------+-------------------------------------
     Reporter:  Hari - 何瑞理        |                    Owner:  nobody
         Type:  Bug                  |                   Status:  new
    Component:  Database layer       |                  Version:  1.11
  (models, ORM)                      |
     Severity:  Normal               |               Resolution:
     Keywords:                       |             Triage Stage:
  postgresql,migration,index         |  Unreviewed
    Has patch:  0                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Description changed by Hari - 何瑞理:

Old description:

> PostgreSQL migration automatically creates an index for fields that set
> db_index=True. An example is SlugField, which sets this property
> implicitly. Thereafter when the unique=True property is added to the
> field the resultant migration script generates an AlterField object to
> apply this unique attribute. The schema editor then incorrectly detects
> this new unique=True attribute to indicate the need to create a like
> index statement on the field which causes an error as it conflicts with
> the already existing index.
>
> The offending piece of code seems to be at
> django/db/backends/postgresql/schema.py:117.
>
> {{{
>  if ((not (old_field.db_index or old_field.unique) and
> new_field.db_index) or
>                 (not old_field.unique and new_field.unique)):
>             like_index_statement = self._create_like_index_sql(model,
> new_field)
>             if like_index_statement is not None:
>                 self.execute(like_index_statement)
> }}}
>
> If it's changed as:
> {{{
>  if (not (old_field.db_index or old_field.unique) and (new_field.db_index
> or new_field.unique)):
>             like_index_statement = self._create_like_index_sql(model,
> new_field)
>             if like_index_statement is not None:
>                 self.execute(like_index_statement)
> }}}
>
> this error won't occur.
>
> PostgreSQL 9.5 introduces 'IF NOT EXISTS' to the CREATE INDEX statement
> which if added to the schema template can also address this problem
> without changing the above logic.
>
> I encountered the problem with SlugField() which implicitly sets db_index
> = True on PostgreSQL 9.4.
>
> Interestingly, I only discovered this when I used django-tenant-schemas
> which adds a thin layer on top of the default Database router setting the
> schema search path before handing over the work to the default router.
> With a vanilla Django installation using default router, the second call
> to create a like index does not throw an error. However, upon reviewing
> the code, the logic does look incorrect.
>
> I'm still investigating to see if this there's more to this than what I
> just described.
New description:

 PostgreSQL migration automatically creates an index for fields that set
 `db_index=True`. An example is `SlugField`, which sets this property
 implicitly. Thereafter when the `unique=True` property is added to the
 field the resultant migration script generates an AlterField object to
 apply this unique attribute. The schema editor then incorrectly detects
 this new `unique=True` attribute to indicate the need to create a like
 index statement on the field which causes an error as it conflicts with
 the already existing index.

 The offending piece of code seems to be at
 django/db/backends/postgresql/schema.py:117.

 {{{
  if ((not (old_field.db_index or old_field.unique) and new_field.db_index)
 or
                 (not old_field.unique and new_field.unique)):
             like_index_statement = self._create_like_index_sql(model,
 new_field)
             if like_index_statement is not None:
                 self.execute(like_index_statement)
 }}}

 If it's changed as:
 {{{
  if (not (old_field.db_index or old_field.unique) and (new_field.db_index
 or new_field.unique)):
             like_index_statement = self._create_like_index_sql(model,
 new_field)
             if like_index_statement is not None:
                 self.execute(like_index_statement)
 }}}

 this error won't occur.

 PostgreSQL 9.5 introduces `IF NOT EXISTS` to the `CREATE INDEX` statement
 which if added to the schema template can also address this problem
 without changing the above logic.

 I encountered the problem with `SlugField()` which implicitly sets
 `db_index=True` on PostgreSQL 9.4.

 Interestingly, I only discovered this when I used `django-tenant-schemas`
 which adds a thin layer on top of the default Database router setting the
 schema search path before handing over the work to the default router.
 With a vanilla Django installation using default router, the second call
 to create a like index does not throw an error. However, upon reviewing
 the code, the logic does look incorrect. Also issuing the duplicate SQL
 statement in PostgreSQL console also throws an error.

 I'm still investigating to see if this there's more to this than what I
 just described.

--

--
Ticket URL: <https://code.djangoproject.com/ticket/28646#comment:1>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/066.2347a87d19f6f339174ae8d4d375c97a%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #28646: Migration calls "CREATE INDEX" when one already exists when 'unique' field attribute is added

Django
In reply to this post by Django
#28646: Migration calls "CREATE INDEX" when one already exists when 'unique' field
attribute is added
-------------------------------------+-------------------------------------
     Reporter:  Hari - 何瑞理        |                    Owner:  nobody
         Type:  Bug                  |                   Status:  new
    Component:  Database layer       |                  Version:  1.11
  (models, ORM)                      |
     Severity:  Normal               |               Resolution:
     Keywords:                       |             Triage Stage:
  postgresql,migration,index         |  Unreviewed
    Has patch:  0                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------

Comment (by Tim Graham):

 That code was last touched in 9356f63a99957f01c14a9788617428a172a29fcb.
 Your proposal results in some tests failures. Can you write a test for
 `tests/schema/tests.py` that demonstrates this issue?

--
Ticket URL: <https://code.djangoproject.com/ticket/28646#comment:2>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/066.698910e7ebaface0d5a496ab646fd290%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #28646: Migration calls "CREATE INDEX" when one already exists when 'unique' field attribute is added

Django
In reply to this post by Django
#28646: Migration calls "CREATE INDEX" when one already exists when 'unique' field
attribute is added
-------------------------------------+-------------------------------------
     Reporter:  Hari - 何瑞理        |                    Owner:  nobody
         Type:  Bug                  |                   Status:  closed
    Component:  Database layer       |                  Version:  1.11
  (models, ORM)                      |               Resolution:
     Severity:  Normal               |  worksforme
     Keywords:                       |             Triage Stage:
  postgresql,migration,index         |  Unreviewed
    Has patch:  0                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Changes (by Tim Graham):

 * status:  new => closed
 * resolution:   => worksforme


Comment:

 I couldn't reproduce this by changing `SlugField()` to
 `SlugField(unique=True)`. Perhaps the bug is in django-tenant-schemas.
 Please reopen if you find that Django is at fault and add more specific
 steps to reproduce.

--
Ticket URL: <https://code.djangoproject.com/ticket/28646#comment:3>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/066.f22bb84e477cdabba8a79737ecb8ca1c%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #28646: Migration calls "CREATE INDEX" when one already exists when 'unique' field attribute is added

Django
In reply to this post by Django
#28646: Migration calls "CREATE INDEX" when one already exists when 'unique' field
attribute is added
-------------------------------------+-------------------------------------
     Reporter:  Hari - 何瑞理        |                    Owner:  nobody
         Type:  Bug                  |                   Status:  new
    Component:  Database layer       |                  Version:  1.11
  (models, ORM)                      |
     Severity:  Normal               |               Resolution:
     Keywords:                       |             Triage Stage:
  postgresql,migration,index         |  Unreviewed
    Has patch:  0                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Changes (by Ariki):

 * status:  closed => new
 * resolution:  worksforme =>


Comment:

 Django tries to create a like index twice and fails when I try to make
 existing SlugField a primary key in a manually written migration. The code
 to reproduce:

 {{{
 import unittest

 from django.db import connection, migrations, models
 from django.db.migrations.state import ProjectState
 from django.test import TestCase


 class ChangePrimaryKeyTest(TestCase):

     def test_change_primary_key(self):
         # Set PostgreSQL messages locale to get error messages
         operation0 = migrations.RunSQL("SET lc_messages = 'C';")
         # Create a model with two fields
         operation1 = migrations.CreateModel(
             'SimpleModel',
             [
                 ("field1", models.SlugField(max_length=20,
 primary_key=True)),
                 ("field2", models.SlugField(max_length=20)),
             ],
         )
         # Drop field1 primary key constraint - this doesn't fail
         operation2 = migrations.AlterField(
             "SimpleModel",
             "field1",
             models.SlugField(max_length=20, primary_key=False),
         )
         # Add a primary key constraint to field2 - this fails
         operation3 = migrations.AlterField(
             "SimpleModel",
             "field2",
             models.SlugField(max_length=20, primary_key=True),
         )

         project_state = ProjectState()
         with connection.schema_editor() as editor:
             new_state = project_state.clone()
             operation0.database_forwards(
                 "migrtest", editor, project_state, new_state)
             operation1.state_forwards("migrtest", new_state)
             operation1.database_forwards(
                 "migrtest", editor, project_state, new_state)
             project_state, new_state = new_state, new_state.clone()
             operation2.state_forwards("migrtest", new_state)
             operation2.database_forwards(
                 "migrtest", editor, project_state, new_state)
             project_state, new_state = new_state, new_state.clone()
             operation3.state_forwards("migrtest", new_state)
             operation3.database_forwards(
                 "migrtest", editor, project_state, new_state)


 }}}

 Error message:

 {{{
 ERROR: test_change_primary_key (migrtest.tests.ChangePrimaryKeyTest)
 ----------------------------------------------------------------------
 Traceback (most recent call last):
   File "/usr/lib/python3.6/site-packages/django/db/backends/utils.py",
 line 65, in execute
     return self.cursor.execute(sql, params)
 psycopg2.ProgrammingError: relation
 "migrtest_simplemodel_field2_972171aa_like" already exists
 }}}

--
Ticket URL: <https://code.djangoproject.com/ticket/28646#comment:4>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/066.ae4fc4f9f3024e2cc4bf006691ff8df7%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #28646: Migration calls "CREATE INDEX" when one already exists when 'unique' field attribute is added (PostgreSQL) (was: Migration calls "CREATE INDEX" when one already exists when 'unique' field attribute is added)

Django
In reply to this post by Django
#28646: Migration calls "CREATE INDEX" when one already exists when 'unique' field
attribute is added (PostgreSQL)
-------------------------------------+-------------------------------------
     Reporter:  Hari - 何瑞理        |                    Owner:  nobody
         Type:  Bug                  |                   Status:  new
    Component:  Migrations           |                  Version:  1.11
     Severity:  Normal               |               Resolution:
     Keywords:                       |             Triage Stage:  Accepted
  postgresql,migration,index         |
    Has patch:  0                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Changes (by Tim Graham):

 * component:  Database layer (models, ORM) => Migrations
 * stage:  Unreviewed => Accepted


Comment:

 I can reproduce as long as the three operations are in the same migration.
 The crash doesn't happen if you put the `AlterField` operations in a
 separate migration.

--
Ticket URL: <https://code.djangoproject.com/ticket/28646#comment:5>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/066.42aafaeae5a61983fad4b062bc10173e%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #28646: Migration calls "CREATE INDEX" when one already exists when 'unique' field attribute is added (PostgreSQL)

Django
In reply to this post by Django
#28646: Migration calls "CREATE INDEX" when one already exists when 'unique' field
attribute is added (PostgreSQL)
-------------------------------------+-------------------------------------
     Reporter:  Hari - 何瑞理        |                    Owner:  Tomer
                                     |  Chachamu
         Type:  Bug                  |                   Status:  assigned
    Component:  Migrations           |                  Version:  1.11
     Severity:  Normal               |               Resolution:
     Keywords:                       |             Triage Stage:  Accepted
  postgresql,migration,index         |
    Has patch:  0                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Changes (by Tomer Chachamu):

 * status:  new => assigned
 * cc: Tomer Chachamu (added)
 * owner:  nobody => Tomer Chachamu


--
Ticket URL: <https://code.djangoproject.com/ticket/28646#comment:6>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/066.40a7f6daa22cbc323fe23a6781e099f1%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #28646: Migration calls "CREATE INDEX" when one already exists when 'unique' field attribute is added (PostgreSQL)

Django
In reply to this post by Django
#28646: Migration calls "CREATE INDEX" when one already exists when 'unique' field
attribute is added (PostgreSQL)
-------------------------------------+-------------------------------------
     Reporter:  Hari - 何瑞理        |                    Owner:  Tomer
                                     |  Chachamu
         Type:  Bug                  |                   Status:  assigned
    Component:  Migrations           |                  Version:  1.11
     Severity:  Normal               |               Resolution:
     Keywords:                       |             Triage Stage:  Accepted
  postgresql,migration,index         |
    Has patch:  0                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------

Comment (by Tomer Chachamu):

 The test case given is incorrect, as Django always uses a fresh schema
 editor for each migration step:

 https://github.com/django/django/blob/master/django/db/migrations/executor.py#L225

 This passes and is similar to other cases in
 {{{migrations/test_operations.py}}}:

 {{{

     def test_change_primary_key(self):
         # Create a model with two fields
         operation1 = migrations.CreateModel(
             'SimpleModel',
             [
                 ("field1", models.SlugField(max_length=20,
 primary_key=True)),
                 ("field2", models.SlugField(max_length=20)),
             ],
         )
         # Drop field1 primary key constraint - this doesn't fail
         operation2 = migrations.AlterField(
             "SimpleModel",
             "field1",
             models.SlugField(max_length=20, primary_key=False),
         )
         # Add a primary key constraint to field2 - this fails
         operation3 = migrations.AlterField(
             "SimpleModel",
             "field2",
             models.SlugField(max_length=20, primary_key=True),
         )

         project_state = ProjectState()
         new_state = project_state.clone()
         operation1.state_forwards("migrtest", new_state)
         with connection.schema_editor() as editor:
             operation1.database_forwards("migrtest", editor,
 project_state, new_state)
         project_state, new_state = new_state, new_state.clone()
         operation2.state_forwards("migrtest", new_state)
         with connection.schema_editor() as editor:
             operation2.database_forwards("migrtest", editor,
 project_state, new_state)
         project_state, new_state = new_state, new_state.clone()
         operation3.state_forwards("migrtest", new_state)
         with connection.schema_editor() as editor:
             operation3.database_forwards("migrtest", editor,
 project_state, new_state)
 }}}

 I'm going to try working off the original bug description to reproduce the
 bug.

--
Ticket URL: <https://code.djangoproject.com/ticket/28646#comment:7>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/066.e98a67a770864ae00821caeab06039f0%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #28646: Migration calls "CREATE INDEX" when one already exists when 'unique' field attribute is added (PostgreSQL)

Django
In reply to this post by Django
#28646: Migration calls "CREATE INDEX" when one already exists when 'unique' field
attribute is added (PostgreSQL)
-------------------------------------+-------------------------------------
     Reporter:  Hari - 何瑞理        |                    Owner:  (none)
         Type:  Bug                  |                   Status:  new
    Component:  Migrations           |                  Version:  1.11
     Severity:  Normal               |               Resolution:
     Keywords:                       |             Triage Stage:  Accepted
  postgresql,migration,index         |
    Has patch:  1                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Changes (by Tomer Chachamu):

 * status:  assigned => new
 * owner:  Tomer Chachamu => (none)
 * has_patch:  0 => 1


Comment:

 I have added a reasonable PR. https://github.com/django/django/pull/9438

 The root cause is that {{{schema_editor.create_model}}} defers the
 creation of indexes, so at any time you can either have indexes on the
 database (can be found using {{{schema_editor._constraint_names}}}),
 deferred or not at all.
 https://github.com/django/django/blob/master/django/db/backends/base/schema.py#L300

 According to the comment they are deferred for SQLite, so one solution
 would be letting schema editors override the behaviour - removing it for
 non-sqlite, or just for postgres. I think it will become more difficult to
 reason about the schema editor in that case, and if done properly the
 deferred sql can also be an optimiser, removing redundant index creations
 and deletions.

 Every place that an index is added ought to check whether the index
 creation is already deferred, and remove the deferred one if so (in favour
 of an immediate one). Every place that an index is removed needs to check
 both deferred indexes and actual indexes.

 We can probably find more bugs by parameterising the schema and migration
 tests to try using separate schema_editors (which flushes deferred SQL to
 the database every step) or sharing schema_editors.

 Not all the lines added have a test backing them up but I think it's ready
 for somebody to have a look at and decide whether the approach is good.

--
Ticket URL: <https://code.djangoproject.com/ticket/28646#comment:8>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/066.7ed3a21f1c13a91b4f99377d3b5b9bbb%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #28646: Migration calls "CREATE INDEX" when one already exists when 'unique' field attribute is added (PostgreSQL)

Django
In reply to this post by Django
#28646: Migration calls "CREATE INDEX" when one already exists when 'unique' field
attribute is added (PostgreSQL)
-------------------------------------+-------------------------------------
     Reporter:  Hari - 何瑞理        |                    Owner:  (none)
         Type:  Bug                  |                   Status:  new
    Component:  Migrations           |                  Version:  1.11
     Severity:  Normal               |               Resolution:
     Keywords:                       |             Triage Stage:  Accepted
  postgresql,migration,index         |
    Has patch:  1                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  1
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Changes (by Carlton Gibson):

 * needs_better_patch:  0 => 1


Comment:

 Comments on PR: we have an error in the boolean logic, not correctly
 distinguishing between the `_unique` and `primary_key` cases.

 (The original suggestion leads to just 3 failures a fix should be simple
 enough...)

--
Ticket URL: <https://code.djangoproject.com/ticket/28646#comment:9>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/066.8edd22b675062033426ba5cbe30d69e0%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #28646: Migration calls "CREATE INDEX" when one already exists when 'unique' field attribute is added (PostgreSQL)

Django
In reply to this post by Django
#28646: Migration calls "CREATE INDEX" when one already exists when 'unique' field
attribute is added (PostgreSQL)
-------------------------------------+-------------------------------------
     Reporter:  Hari - 何瑞理        |                    Owner:  (none)
         Type:  Bug                  |                   Status:  new
    Component:  Migrations           |                  Version:  1.11
     Severity:  Normal               |               Resolution:
     Keywords:                       |             Triage Stage:  Accepted
  postgresql,migration,index,#djangocph|
    Has patch:  1                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  1
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Changes (by Carlton Gibson):

 * keywords:  postgresql,migration,index =>
     postgresql,migration,index,#djangocph


Comment:

 I'm going to mark this for #djangocph for the sprint in Copenhagen.

 Whilst it's right in the heart of the migration framework, I think it
 should be an easy fix.

 Here's the GitHub permalink to the problem `if` check:
 https://github.com/django/django/blob/fb8fd535c0f47cffb4da0c5900f3f66e1ec8d432/django/db/backends/postgresql/schema.py#L124-L126

 If you apply the original suggested fix you get a small number of failures
 (3 I think). These relate to needing to add an index due to a  `unique`
 flag being added. That's the last `or` in the problem `if`.

 The test from https://github.com/django/django/pull/9438 checks the new
 problem behaviour. (Can it live with the tests that fail if you apply the
 suggested fix?)

 That new test fails because the `unique` property is essentially `_unique
 or primary_key`, which is too wide. As I said above, using
 `new_field._unique` was enough to make the test pass.

 The task here is to go through that and make sure it's correct. Make sure
 the test is in the right place. Add a comment in the code (if it's
 needed). Maybe a release note etc.

--
Ticket URL: <https://code.djangoproject.com/ticket/28646#comment:10>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/066.471d33eb2d48d82ca3ff163defc1d99c%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.