[Django] #30754: Partial indexes break future migrations in sqlite

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

[Django] #30754: Partial indexes break future migrations in sqlite

Django
#30754: Partial indexes break future migrations in sqlite
--------------------------------------+------------------------
               Reporter:  cuu508      |          Owner:  nobody
                   Type:  Bug         |         Status:  new
              Component:  Migrations  |        Version:  2.2
               Severity:  Normal      |       Keywords:
           Triage Stage:  Unreviewed  |      Has patch:  0
    Needs documentation:  0           |    Needs tests:  0
Patch needs improvement:  0           |  Easy pickings:  0
                  UI/UX:  0           |
--------------------------------------+------------------------
 How to reproduce:

 1. Create a dummy "Question" model (lifted from Django's tutorial)
 2. Add a partial index on one of its fields, create a migration and apply
 it
 3. Add another field to the model, create the migration (works) and apply
 it (throws an error)

 The error I get looks like this:

 {{{
 $ ./manage.py migrate
 Operations to perform:
   Apply all migrations: admin, auth, contenttypes, polls, sessions
 Running migrations:
   Applying polls.0003_question_hint...Traceback (most recent call last):
   File "/tmp/htemp/lib/python3.7/site-
 packages/django/db/backends/utils.py", line 84, in _execute
     return self.cursor.execute(sql, params)
   File "/tmp/htemp/lib/python3.7/site-
 packages/django/db/backends/sqlite3/base.py", line 383, in execute
     return Database.Cursor.execute(self, query, params)
 sqlite3.OperationalError: no such column:
 new__polls_question.question_text

 The above exception was the direct cause of the following exception:

 Traceback (most recent call last):
   File "./manage.py", line 21, in <module>
     main()
   File "./manage.py", line 17, in main
     execute_from_command_line(sys.argv)
   File "/tmp/htemp/lib/python3.7/site-
 packages/django/core/management/__init__.py", line 381, in
 execute_from_command_line
     utility.execute()
   File "/tmp/htemp/lib/python3.7/site-
 packages/django/core/management/__init__.py", line 375, in execute
     self.fetch_command(subcommand).run_from_argv(self.argv)
   File "/tmp/htemp/lib/python3.7/site-
 packages/django/core/management/base.py", line 323, in run_from_argv
     self.execute(*args, **cmd_options)
   File "/tmp/htemp/lib/python3.7/site-
 packages/django/core/management/base.py", line 364, in execute
     output = self.handle(*args, **options)
   File "/tmp/htemp/lib/python3.7/site-
 packages/django/core/management/base.py", line 83, in wrapped
     res = handle_func(*args, **kwargs)
   File "/tmp/htemp/lib/python3.7/site-
 packages/django/core/management/commands/migrate.py", line 234, in handle
     fake_initial=fake_initial,
   File "/tmp/htemp/lib/python3.7/site-
 packages/django/db/migrations/executor.py", line 117, in migrate
     state = self._migrate_all_forwards(state, plan, full_plan, fake=fake,
 fake_initial=fake_initial)
   File "/tmp/htemp/lib/python3.7/site-
 packages/django/db/migrations/executor.py", line 147, in
 _migrate_all_forwards
     state = self.apply_migration(state, migration, fake=fake,
 fake_initial=fake_initial)
   File "/tmp/htemp/lib/python3.7/site-
 packages/django/db/migrations/executor.py", line 245, in apply_migration
     state = migration.apply(state, schema_editor)
   File "/tmp/htemp/lib/python3.7/site-
 packages/django/db/migrations/migration.py", line 124, in apply
     operation.database_forwards(self.app_label, schema_editor, old_state,
 project_state)
   File "/tmp/htemp/lib/python3.7/site-
 packages/django/db/migrations/operations/fields.py", line 112, in
 database_forwards
     field,
   File "/tmp/htemp/lib/python3.7/site-
 packages/django/db/backends/sqlite3/schema.py", line 327, in add_field
     self._remake_table(model, create_field=field)
   File "/tmp/htemp/lib/python3.7/site-
 packages/django/db/backends/sqlite3/schema.py", line 300, in _remake_table
     self.execute(sql)
   File "/tmp/htemp/lib/python3.7/site-
 packages/django/db/backends/base/schema.py", line 137, in execute
     cursor.execute(sql, params)
   File "/tmp/htemp/lib/python3.7/site-
 packages/django/db/backends/utils.py", line 99, in execute
     return super().execute(sql, params)
   File "/tmp/htemp/lib/python3.7/site-
 packages/django/db/backends/utils.py", line 67, in execute
     return self._execute_with_wrappers(sql, params, many=False,
 executor=self._execute)
   File "/tmp/htemp/lib/python3.7/site-
 packages/django/db/backends/utils.py", line 76, in _execute_with_wrappers
     return executor(sql, params, many, context)
   File "/tmp/htemp/lib/python3.7/site-
 packages/django/db/backends/utils.py", line 84, in _execute
     return self.cursor.execute(sql, params)
   File "/tmp/htemp/lib/python3.7/site-packages/django/db/utils.py", line
 89, in __exit__
     raise dj_exc_value.with_traceback(traceback) from exc_value
   File "/tmp/htemp/lib/python3.7/site-
 packages/django/db/backends/utils.py", line 84, in _execute
     return self.cursor.execute(sql, params)
   File "/tmp/htemp/lib/python3.7/site-
 packages/django/db/backends/sqlite3/base.py", line 383, in execute
     return Database.Cursor.execute(self, query, params)
 django.db.utils.OperationalError: no such column:
 new__polls_question.question_text
 }}}

 Note that the migration creating an partial index works. It's the *next*
 migration that fails.

 For me, this happens only with SQLite, no errors with PostgreSQL. Also no
 problems with MySQL, which does not support partial indexes.
 If I remove the `condition` clause (i.e., create a regular index instead
 of an partial index) then it works fine.

 Here's an isolated test-case, I've added the three steps in 3 separate
 commits: https://github.com/cuu508/sqlite_partial_indexes

 I patched `django/db/backends/sqlite3/base.py` to print SQL queries to
 stdout. The relevant part:

 {{{
 DROP TABLE "polls_question"
 ALTER TABLE "new__polls_question" RENAME TO "polls_question"
 CREATE INDEX "polls_nonempty_pub_date" ON "polls_question" ("pub_date")
 WHERE NOT ("new__polls_question"."question_text" = '')
 Traceback (most recent call last):
   File "/tmp/htemp/lib/python3.7/site-
 packages/django/db/backends/utils.py", line 84, in _execute
     return self.cursor.execute(sql, params)
   File "/tmp/htemp/lib/python3.7/site-
 packages/django/db/backends/sqlite3/base.py", line 420, in execute
     return Database.Cursor.execute(self, query, params)
 sqlite3.OperationalError: no such column:
 new__polls_question.question_text
 }}}

 It appears to be renaming the table, and then trying to use it by its old
 name.
 Apologies if this is already reported – couldn't find a similar ticket
 with a quick search for "sqlite".

--
Ticket URL: <https://code.djangoproject.com/ticket/30754>
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 view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/049.12e1a4d4845c76c0c299a086cb1f7859%40djangoproject.com.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #30754: Partial indexes break future migrations in sqlite

Django
#30754: Partial indexes break future migrations in sqlite
----------------------------+------------------------------------
     Reporter:  cuu508      |                    Owner:  nobody
         Type:  Bug         |                   Status:  new
    Component:  Migrations  |                  Version:  2.2
     Severity:  Normal      |               Resolution:
     Keywords:  sqlite      |             Triage Stage:  Accepted
    Has patch:  0           |      Needs documentation:  0
  Needs tests:  0           |  Patch needs improvement:  0
Easy pickings:  0           |                    UI/UX:  0
----------------------------+------------------------------------
Changes (by Simon Charette):

 * keywords:   => sqlite
 * stage:  Unreviewed => Accepted


Comment:

 This is probably another edge cased missed by the create/rename table
 workaround on SQLite to emulate `ALTER TABLE` wrt to partial indices.

 If possible we should simply not include the table alias in the partial
 index predicate else I suspect we'll need to defer index re-creation on
 the table '''after''' the rename.

 I haven't reproduced but I feel confident accepting the ticket based on
 the detailed report and reduced reproduction case.

--
Ticket URL: <https://code.djangoproject.com/ticket/30754#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 view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/064.55703a55bdb9d5c10599dfb0d75452d0%40djangoproject.com.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #30754: Partial indexes break future migrations in sqlite

Django
In reply to this post by Django
#30754: Partial indexes break future migrations in sqlite
---------------------------------+------------------------------------
     Reporter:  cuu508           |                    Owner:  nobody
         Type:  Bug              |                   Status:  new
    Component:  Migrations       |                  Version:  2.2
     Severity:  Release blocker  |               Resolution:
     Keywords:  sqlite           |             Triage Stage:  Accepted
    Has patch:  0                |      Needs documentation:  0
  Needs tests:  0                |  Patch needs improvement:  0
Easy pickings:  0                |                    UI/UX:  0
---------------------------------+------------------------------------
Changes (by felixxm):

 * severity:  Normal => Release blocker


Comment:

 Bumping to a release blocker because partial indexes are a new feature.

--
Ticket URL: <https://code.djangoproject.com/ticket/30754#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 view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/064.5beb1bfbdfd986d42fe4ef268e95cfcb%40djangoproject.com.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #30754: Partial indexes break future migrations in sqlite

Django
In reply to this post by Django
#30754: Partial indexes break future migrations in sqlite
-------------------------------------+-------------------------------------
     Reporter:  Pēteris Caune        |                    Owner:  Simon
                                     |  Charette
         Type:  Bug                  |                   Status:  assigned
    Component:  Migrations           |                  Version:  2.2
     Severity:  Release blocker      |               Resolution:
     Keywords:  sqlite               |             Triage Stage:  Accepted
    Has patch:  0                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Changes (by Simon Charette):

 * owner:  nobody => Simon Charette
 * status:  new => assigned


--
Ticket URL: <https://code.djangoproject.com/ticket/30754#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 view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/064.6b00bd64838a51e37cc638d4e3a95ae1%40djangoproject.com.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #30754: Partial indexes break future migrations in sqlite

Django
In reply to this post by Django
#30754: Partial indexes break future migrations in sqlite
-------------------------------------+-------------------------------------
     Reporter:  Pēteris Caune        |                    Owner:  Simon
                                     |  Charette
         Type:  Bug                  |                   Status:  assigned
    Component:  Migrations           |                  Version:  2.2
     Severity:  Release blocker      |               Resolution:
     Keywords:  sqlite               |             Triage Stage:  Accepted
    Has patch:  1                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Changes (by Simon Charette):

 * has_patch:  0 => 1


Comment:

 The issue was effectively table aliases inclusion for columns in the
 `WHERE` clause because of the rename during `_remake_table`.

 Pēteris, could you confirm this patch effectively addresses your issue?
 I've tested it out locally but schema alteration on SQLite is particularly
 gnarly from versions to versions.

--
Ticket URL: <https://code.djangoproject.com/ticket/30754#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 view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/064.8caf909faa4ab043f137858d57396ba2%40djangoproject.com.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #30754: Partial indexes break future migrations in sqlite

Django
In reply to this post by Django
#30754: Partial indexes break future migrations in sqlite
-------------------------------------+-------------------------------------
     Reporter:  Pēteris Caune        |                    Owner:  Simon
                                     |  Charette
         Type:  Bug                  |                   Status:  assigned
    Component:  Migrations           |                  Version:  2.2
     Severity:  Release blocker      |               Resolution:
     Keywords:  sqlite               |             Triage Stage:  Accepted
    Has patch:  1                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------

Comment (by Pēteris Caune):

 Hello Simon, tested your patch with the minimal reproduction case and also
 in the project where I first encountered the issue. Migrations now run
 without errors in both!

--
Ticket URL: <https://code.djangoproject.com/ticket/30754#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 view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/064.19917ebaaa60f7c528bfaac9dc0a3484%40djangoproject.com.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #30754: Partial indexes break future migrations in sqlite

Django
In reply to this post by Django
#30754: Partial indexes break future migrations in sqlite
-------------------------------------+-------------------------------------
     Reporter:  Pēteris Caune        |                    Owner:  Simon
                                     |  Charette
         Type:  Bug                  |                   Status:  closed
    Component:  Migrations           |                  Version:  2.2
     Severity:  Release blocker      |               Resolution:  fixed
     Keywords:  sqlite               |             Triage Stage:  Accepted
    Has patch:  1                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Changes (by Mariusz Felisiak <felisiak.mariusz@…>):

 * status:  assigned => closed
 * resolution:   => fixed


Comment:

 In [changeset:"34decdebf157b6f05836009cc1967f74ee541fdf" 34decdeb]:
 {{{
 #!CommitTicketReference repository=""
 revision="34decdebf157b6f05836009cc1967f74ee541fdf"
 Fixed #30754 -- Prevented inclusion of aliases in partial index
 conditions.

 SQLite doesn't repoint table aliases in partial index conditions on table
 rename which breaks the documented table alteration procedure.

 Thanks Pēteris Caune for the report.
 }}}

--
Ticket URL: <https://code.djangoproject.com/ticket/30754#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 view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/064.b2b0bc264dfcf9bad7d94e453959ae2c%40djangoproject.com.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #30754: Partial indexes break future migrations in sqlite

Django
In reply to this post by Django
#30754: Partial indexes break future migrations in sqlite
-------------------------------------+-------------------------------------
     Reporter:  Pēteris Caune        |                    Owner:  Simon
                                     |  Charette
         Type:  Bug                  |                   Status:  closed
    Component:  Migrations           |                  Version:  2.2
     Severity:  Release blocker      |               Resolution:  fixed
     Keywords:  sqlite               |             Triage Stage:  Accepted
    Has patch:  1                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------

Comment (by Mariusz Felisiak <felisiak.mariusz@…>):

 In [changeset:"964dd4f4f208722d8993a35c1ff047d353cea1ea" 964dd4f]:
 {{{
 #!CommitTicketReference repository=""
 revision="964dd4f4f208722d8993a35c1ff047d353cea1ea"
 [2.2.x] Fixed #30754 -- Prevented inclusion of aliases in partial index
 conditions.

 SQLite doesn't repoint table aliases in partial index conditions on table
 rename which breaks the documented table alteration procedure.

 Thanks Pēteris Caune for the report.

 Backport of 34decdebf157b6f05836009cc1967f74ee541fdf from master
 }}}

--
Ticket URL: <https://code.djangoproject.com/ticket/30754#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 view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/064.73194808b8c7a7519623ff964585ce54%40djangoproject.com.