[Django] #23909: RunSQL calls execute with params None, params used in map
Django
----------------------------------------------+------------------------
Reporter: yarbelk | Owner: nobody
Type: Bug | Status: new
Component: Database layer (models, ORM) | Version: 1.7
Severity: Normal | Keywords: migrations
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------------------+------------------------
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/23909>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: yarbelk | Owner: nobody
Type: Bug | Status: new
(models, ORM) | Resolution:
Severity: Release blocker | Triage Stage: Accepted
Keywords: migrations | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by claudep):
* needs_better_patch: => 0
* needs_docs: => 0
* severity: Normal => Release blocker
* needs_tests: => 0
* stage: Unreviewed => Accepted
Comment:
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/23909#comment:1>
Django
Reporter: yarbelk | Owner: nobody
Type: Bug | Status: new
(models, ORM) | Resolution:
Severity: Release blocker | Triage Stage: Accepted
Keywords: migrations | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by MarkusH):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/23909#comment:2>
Django
Reporter: yarbelk | Owner: nobody
Type: Bug | Status: new
(models, ORM) | Resolution:
Severity: Release blocker | Triage Stage: Accepted
Keywords: migrations | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by claudep):
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
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/23909#comment:3>
Django
Reporter: yarbelk | Owner: nobody
Type: Bug | Status: new
(models, ORM) | Resolution:
Severity: Release blocker | Triage Stage: Accepted
Keywords: migrations | Needs documentation: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
* has_patch: 0 => 1
Comment:
https://github.com/django/django/pull/3656
--
Ticket URL: <https://code.djangoproject.com/ticket/23909#comment:4>
Django
Reporter: yarbelk | Owner: nobody
Type: Bug | Status: closed
Component: Database layer | Version: 1.7
Severity: Release blocker | Triage Stage: Accepted
Keywords: migrations | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"e11c6fd2184a9e7b23c777e5824f9ded6221d905"]:
{{{
#!CommitTicketReference repository=""
revision="e11c6fd2184a9e7b23c777e5824f9ded6221d905"
Fixed #23909 -- Prevented crash when collecting SQL for RunSQL
Thanks James Rivett-Carnac for the report and Markus Holtermann
for the review.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/23909#comment:5>
Django
Reporter: yarbelk | Owner: nobody
Type: Bug | Status: closed
Component: Database layer | Version: 1.7
(models, ORM) | Resolution: fixed
Severity: Release blocker | Triage Stage: Accepted
Keywords: migrations | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by Claude Paroz <claude@…>):
In [changeset:"3a42d9730cbb07ffbb983791e631f5d0a6746f68"]:
{{{
#!CommitTicketReference repository=""
revision="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.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/23909#comment:6>