Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#23909 closed Bug (fixed)

RunSQL calls execute with params None, params used in map

Reported by: James Rivett-Carnac Owned by: nobody
Component: Database layer (models, ORM) Version: 1.7
Severity: Release blocker Keywords: migrations
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

with a migration operation:

    migrations.RunSQL(
        """CREATE TABLE ...; CREATE TABLE...; """,
        """DROP TABLE ...; DROP TABLE...;"""",
        state_operations=[list of the equivalent migrations])

calling migrate on this gives me a TypeError on line in django.db.backends.schema.execute():96

    self.collected_sql.append((sql % tuple(map(self.quote_value, params))) + ';')

But this is being called by django.db.migrations.operations.special:69

    schema_editor.execute(statement, params=None)

This will always fail if it is called so - None never being itterable. the method signature for django.db.backends.schema.execute(sql, params=[]) should be enough rather than calling with None.

Change History (6)

comment:1 Changed 6 years ago by Claude Paroz

Severity: NormalRelease blocker
Triage Stage: UnreviewedAccepted

It's important to be able to keep params=None as it will have an impact on the parameter substitution in the sql string. Therefore I think that it's the self.collected_sql.append line that has to be fixed to take the None value into account.

comment:2 Changed 6 years ago by Markus Holtermann

Depending on your database backend (SQLite and MySQL for sure; not sure about Oracle) you need to have sqlparse installed.

A related problem was tackled in #23426. Django 1.8 will add extended support for multiple statements including parameters.

I'd close the ticket as "invalid" or "needsinfo", because I'm not sure which backend you are using.

comment:3 Changed 6 years ago by Claude Paroz

To reproduce the error in current tests:

diff --git a/tests/migrations/test_operations.py b/tests/migrations/test_operations.py
index 32f70e8..08a5600 100644
--- a/tests/migrations/test_operations.py
+++ b/tests/migrations/test_operations.py
@@ -1292,7 +1292,7 @@ class OperationTests(OperationTestBase):
         # Make sure there's no table
         self.assertTableNotExists("i_love_ponies")
         # Test the database alteration
-        with connection.schema_editor() as editor:
+        with connection.schema_editor(collect_sql=True) as editor:
             operation.database_forwards("test_runsql", editor, project_state, new_state)
         self.assertTableExists("i_love_ponies")
         # Make sure all the SQL was processed

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

Resolution: fixed
Status: newclosed

In e11c6fd2184a9e7b23c777e5824f9ded6221d905:

Fixed #23909 -- Prevented crash when collecting SQL for RunSQL

Thanks James Rivett-Carnac for the report and Markus Holtermann
for the review.

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

In 3a42d9730cbb07ffbb983791e631f5d0a6746f68:

[1.7.x] Fixed #23909 -- Prevented crash when collecting SQL for RunSQL

Thanks James Rivett-Carnac for the report and Markus Holtermann
for the review.
Backport of e11c6fd21 from master.

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