Fixed #3214 -- Stopped parsing SQL with regex.

Avoided introducing a new regex-based SQL splitter in the migrations
framework, before we're bound by backwards compatibility.

Adapted this change to the legacy "initial SQL data" feature, even
though it's already deprecated, in order to facilitate the transition
to migrations.

sqlparse becomes mandatory for RunSQL on some databases (all but
PostgreSQL). There's no API to provide a single statement and tell
Django not to attempt splitting. Since we have a more robust splitting
implementation, that seems like a good tradeoff. It's easier to add a
new keyword argument later if necessary than to remove one.

Many people contributed to both tickets, thank you all, and especially
Claude for the review.

Refs #22401.
This commit is contained in:
Aymeric Augustin 2014-04-26 10:22:48 +02:00
parent 2128b3a688
commit 8b5b199e20
11 changed files with 77 additions and 38 deletions

View File

@ -155,6 +155,7 @@ def sql_all(app_config, style, connection):
def _split_statements(content): def _split_statements(content):
# Private API only called from code that emits a RemovedInDjango19Warning.
comment_re = re.compile(r"^((?:'[^']*'|[^'])*?)--.*$") comment_re = re.compile(r"^((?:'[^']*'|[^'])*?)--.*$")
statements = [] statements = []
statement = [] statement = []
@ -202,9 +203,7 @@ def custom_sql_for_model(model, style, connection):
for sql_file in sql_files: for sql_file in sql_files:
if os.path.exists(sql_file): if os.path.exists(sql_file):
with codecs.open(sql_file, 'r' if six.PY3 else 'U', encoding=settings.FILE_CHARSET) as fp: with codecs.open(sql_file, 'r' if six.PY3 else 'U', encoding=settings.FILE_CHARSET) as fp:
# Some backends can't execute more than one SQL statement at a time, output.extend(connection.ops.prepare_sql_script(fp.read(), _allow_fallback=True))
# so split into separate statements.
output.extend(_split_statements(fp.read()))
return output return output

View File

@ -1,5 +1,6 @@
import datetime import datetime
import time import time
import warnings
try: try:
from django.utils.six.moves import _thread as thread from django.utils.six.moves import _thread as thread
@ -16,6 +17,7 @@ from django.db.backends.signals import connection_created
from django.db.backends import utils from django.db.backends import utils
from django.db.transaction import TransactionManagementError from django.db.transaction import TransactionManagementError
from django.db.utils import DatabaseError, DatabaseErrorWrapper, ProgrammingError from django.db.utils import DatabaseError, DatabaseErrorWrapper, ProgrammingError
from django.utils.deprecation import RemovedInDjango19Warning
from django.utils.functional import cached_property from django.utils.functional import cached_property
from django.utils import six from django.utils import six
from django.utils import timezone from django.utils import timezone
@ -599,6 +601,10 @@ class BaseDatabaseFeatures(object):
# Does 'a' LIKE 'A' match? # Does 'a' LIKE 'A' match?
has_case_insensitive_like = True has_case_insensitive_like = True
# Does the backend require the sqlparse library for splitting multi-line
# statements before executing them?
requires_sqlparse_for_splitting = True
def __init__(self, connection): def __init__(self, connection):
self.connection = connection self.connection = connection
@ -867,6 +873,34 @@ class BaseDatabaseOperations(object):
""" """
return 'DEFAULT' return 'DEFAULT'
def prepare_sql_script(self, sql, _allow_fallback=False):
"""
Takes a SQL script that may contain multiple lines and returns a list
of statements to feed to successive cursor.execute() calls.
Since few databases are able to process raw SQL scripts in a single
cursor.execute() call and PEP 249 doesn't talk about this use case,
the default implementation is conservative.
"""
# Remove _allow_fallback and keep only 'return ...' in Django 1.9.
try:
# This import must stay inside the method because it's optional.
import sqlparse
except ImportError:
if _allow_fallback:
# Without sqlparse, fall back to the legacy (and buggy) logic.
warnings.warn(
"Providing intial SQL data on a %s database will require "
"sqlparse in Django 1.9." % self.connection.vendor,
RemovedInDjango19Warning)
from django.core.management.sql import _split_statements
return _split_statements(sql)
else:
raise
else:
return [sqlparse.format(statement, strip_comments=True)
for statement in sqlparse.split(sql) if statement]
def process_clob(self, value): def process_clob(self, value):
""" """
Returns the value of a CLOB column, for backends that return a locator Returns the value of a CLOB column, for backends that return a locator

View File

@ -58,6 +58,7 @@ class DatabaseFeatures(BaseDatabaseFeatures):
nulls_order_largest = True nulls_order_largest = True
closed_cursor_error_class = InterfaceError closed_cursor_error_class = InterfaceError
has_case_insensitive_like = False has_case_insensitive_like = False
requires_sqlparse_for_splitting = False
class DatabaseWrapper(BaseDatabaseWrapper): class DatabaseWrapper(BaseDatabaseWrapper):

View File

@ -93,6 +93,9 @@ class DatabaseOperations(BaseDatabaseOperations):
def no_limit_value(self): def no_limit_value(self):
return None return None
def prepare_sql_script(self, sql, _allow_fallback=False):
return [sql]
def quote_name(self, name): def quote_name(self, name):
if name.startswith('"') and name.endswith('"'): if name.startswith('"') and name.endswith('"'):
return name # Quoting once is enough. return name # Quoting once is enough.

View File

@ -1,4 +1,3 @@
import re
from .base import Operation from .base import Operation
@ -43,20 +42,16 @@ class SeparateDatabaseAndState(Operation):
class RunSQL(Operation): class RunSQL(Operation):
""" """
Runs some raw SQL - a single statement by default, but it will attempt Runs some raw SQL. A reverse SQL statement may be provided.
to parse and split it into multiple statements if multiple=True.
A reverse SQL statement may be provided.
Also accepts a list of operations that represent the state change effected Also accepts a list of operations that represent the state change effected
by this SQL change, in case it's custom column/table creation/deletion. by this SQL change, in case it's custom column/table creation/deletion.
""" """
def __init__(self, sql, reverse_sql=None, state_operations=None, multiple=False): def __init__(self, sql, reverse_sql=None, state_operations=None):
self.sql = sql self.sql = sql
self.reverse_sql = reverse_sql self.reverse_sql = reverse_sql
self.state_operations = state_operations or [] self.state_operations = state_operations or []
self.multiple = multiple
@property @property
def reversible(self): def reversible(self):
@ -66,30 +61,15 @@ class RunSQL(Operation):
for state_operation in self.state_operations: for state_operation in self.state_operations:
state_operation.state_forwards(app_label, state) state_operation.state_forwards(app_label, state)
def _split_sql(self, sql):
regex = r"(?mx) ([^';]* (?:'[^']*'[^';]*)*)"
comment_regex = r"(?mx) (?:^\s*$)|(?:--.*$)"
# First, strip comments
sql = "\n".join([x.strip().replace("%", "%%") for x in re.split(comment_regex, sql) if x.strip()])
# Now get each statement
for st in re.split(regex, sql)[1:][::2]:
yield st
def database_forwards(self, app_label, schema_editor, from_state, to_state): def database_forwards(self, app_label, schema_editor, from_state, to_state):
if self.multiple: statements = schema_editor.connection.ops.prepare_sql_script(self.sql)
statements = self._split_sql(self.sql)
else:
statements = [self.sql]
for statement in statements: for statement in statements:
schema_editor.execute(statement) schema_editor.execute(statement)
def database_backwards(self, app_label, schema_editor, from_state, to_state): def database_backwards(self, app_label, schema_editor, from_state, to_state):
if self.reverse_sql is None: if self.reverse_sql is None:
raise NotImplementedError("You cannot reverse this operation") raise NotImplementedError("You cannot reverse this operation")
if self.multiple: statements = schema_editor.connection.ops.prepare_sql_script(self.reverse_sql)
statements = self._split_sql(self.reverse_sql)
else:
statements = [self.reverse_sql]
for statement in statements: for statement in statements:
schema_editor.execute(statement) schema_editor.execute(statement)

View File

@ -166,6 +166,7 @@ dependencies:
* memcached_, plus a :ref:`supported Python binding <memcached>` * memcached_, plus a :ref:`supported Python binding <memcached>`
* gettext_ (:ref:`gettext_on_windows`) * gettext_ (:ref:`gettext_on_windows`)
* selenium_ * selenium_
* sqlparse_
You can find these dependencies in `pip requirements files`_ inside the You can find these dependencies in `pip requirements files`_ inside the
``tests/requirements`` directory of the Django source tree and install them ``tests/requirements`` directory of the Django source tree and install them
@ -197,6 +198,7 @@ associated tests will be skipped.
.. _memcached: http://memcached.org/ .. _memcached: http://memcached.org/
.. _gettext: http://www.gnu.org/software/gettext/manual/gettext.html .. _gettext: http://www.gnu.org/software/gettext/manual/gettext.html
.. _selenium: https://pypi.python.org/pypi/selenium .. _selenium: https://pypi.python.org/pypi/selenium
.. _sqlparse: https://pypi.python.org/pypi/sqlparse
.. _pip requirements files: http://www.pip-installer.org/en/latest/cookbook.html#requirements-files .. _pip requirements files: http://www.pip-installer.org/en/latest/cookbook.html#requirements-files
Code coverage Code coverage

View File

@ -167,25 +167,23 @@ Changes a field's name (and, unless ``db_column`` is set, its column name).
Special Operations Special Operations
================== ==================
.. _operation-run-sql:
RunSQL RunSQL
------ ------
:: ::
RunSQL(sql, reverse_sql=None, state_operations=None, multiple=False) RunSQL(sql, reverse_sql=None, state_operations=None)
Allows running of arbitrary SQL on the database - useful for more advanced Allows running of arbitrary SQL on the database - useful for more advanced
features of database backends that Django doesn't support directly, like features of database backends that Django doesn't support directly, like
partial indexes. partial indexes.
``sql``, and ``reverse_sql`` if provided, should be strings of SQL to run on the ``sql``, and ``reverse_sql`` if provided, should be strings of SQL to run on
database. They will be passed to the database as a single SQL statement unless the database. On most database backends (all but PostgreSQL), Django will
``multiple`` is set to ``True``, in which case they will be split into separate split the SQL into individual statements prior to executing them. This
statements manually by the operation before being passed through. requires installing the sqlparse_ Python library.
In some extreme cases, the built-in statement splitter may not be able to split
correctly, in which case you should manually split the SQL into multiple calls
to ``RunSQL``.
The ``state_operations`` argument is so you can supply operations that are The ``state_operations`` argument is so you can supply operations that are
equivalent to the SQL in terms of project state; for example, if you are equivalent to the SQL in terms of project state; for example, if you are
@ -194,6 +192,7 @@ operation here so that the autodetector still has an up-to-date state of the
model (otherwise, when you next run ``makemigrations``, it won't see any model (otherwise, when you next run ``makemigrations``, it won't see any
operation that adds that field and so will try to run it again). operation that adds that field and so will try to run it again).
.. _sqlparse: https://pypi.python.org/pypi/sqlparse
.. _operation-run-python: .. _operation-run-python:

View File

@ -636,6 +636,14 @@ Management Commands
* :djadmin:`collectstatic` command with symlink option is now supported on * :djadmin:`collectstatic` command with symlink option is now supported on
Windows NT 6 (Windows Vista and newer). Windows NT 6 (Windows Vista and newer).
* :ref:`initial-sql` now works better if the sqlparse_ Python library is
installed.
Note that it's deprecated in favor of the :ref:`RunSQL <operation-run-sql>`
operation of migrations, which benefits from the improved behavior.
.. _sqlparse: https://pypi.python.org/pypi/sqlparse
Models Models
^^^^^^ ^^^^^^

View File

@ -27,7 +27,6 @@ class InitialSQLTests(TestCase):
""" """
connection = connections[DEFAULT_DB_ALIAS] connection = connections[DEFAULT_DB_ALIAS]
custom_sql = custom_sql_for_model(Simple, no_style(), connection) custom_sql = custom_sql_for_model(Simple, no_style(), connection)
self.assertEqual(len(custom_sql), 9)
with connection.cursor() as cursor: with connection.cursor() as cursor:
for sql in custom_sql: for sql in custom_sql:
cursor.execute(sql) cursor.execute(sql)

View File

@ -1,5 +1,10 @@
import unittest import unittest
try:
import sqlparse
except ImportError:
sqlparse = None
from django.db import connection, migrations, models, router from django.db import connection, migrations, models, router
from django.db.migrations.migration import Migration from django.db.migrations.migration import Migration
from django.db.migrations.state import ProjectState from django.db.migrations.state import ProjectState
@ -640,6 +645,7 @@ class OperationTests(MigrationTestBase):
operation.database_backwards("test_alinto", editor, new_state, project_state) operation.database_backwards("test_alinto", editor, new_state, project_state)
self.assertIndexNotExists("test_alinto_pony", ["pink", "weight"]) self.assertIndexNotExists("test_alinto_pony", ["pink", "weight"])
@unittest.skipIf(sqlparse is None and connection.features.requires_sqlparse_for_splitting, "Missing sqlparse")
def test_run_sql(self): def test_run_sql(self):
""" """
Tests the RunSQL operation. Tests the RunSQL operation.
@ -647,7 +653,10 @@ class OperationTests(MigrationTestBase):
project_state = self.set_up_test_model("test_runsql") project_state = self.set_up_test_model("test_runsql")
# Create the operation # Create the operation
operation = migrations.RunSQL( operation = migrations.RunSQL(
"CREATE TABLE i_love_ponies (id int, special_thing int)", # Use a multi-line string with a commment to test splitting on SQLite and MySQL respectively
"CREATE TABLE i_love_ponies (id int, special_thing int);\n"
"INSERT INTO i_love_ponies (id, special_thing) VALUES (1, 42); -- this is magic!\n"
"INSERT INTO i_love_ponies (id, special_thing) VALUES (2, 51);\n",
"DROP TABLE i_love_ponies", "DROP TABLE i_love_ponies",
state_operations=[migrations.CreateModel("SomethingElse", [("id", models.AutoField(primary_key=True))])], state_operations=[migrations.CreateModel("SomethingElse", [("id", models.AutoField(primary_key=True))])],
) )
@ -661,6 +670,10 @@ class OperationTests(MigrationTestBase):
with connection.schema_editor() as editor: with connection.schema_editor() as editor:
operation.database_forwards("test_runsql", editor, project_state, new_state) operation.database_forwards("test_runsql", editor, project_state, new_state)
self.assertTableExists("i_love_ponies") self.assertTableExists("i_love_ponies")
# Make sure all the SQL was processed
with connection.cursor() as cursor:
cursor.execute("SELECT COUNT(*) FROM i_love_ponies")
self.assertEqual(cursor.fetchall()[0][0], 2)
# And test reversal # And test reversal
self.assertTrue(operation.reversible) self.assertTrue(operation.reversible)
with connection.schema_editor() as editor: with connection.schema_editor() as editor:

View File

@ -5,3 +5,4 @@ Pillow
PyYAML PyYAML
pytz > dev pytz > dev
selenium selenium
sqlparse