diff options
| -rw-r--r-- | django/core/management/sql.py | 5 | ||||
| -rw-r--r-- | django/db/backends/__init__.py | 34 | ||||
| -rw-r--r-- | django/db/backends/postgresql_psycopg2/base.py | 1 | ||||
| -rw-r--r-- | django/db/backends/postgresql_psycopg2/operations.py | 3 | ||||
| -rw-r--r-- | django/db/migrations/operations/special.py | 28 | ||||
| -rw-r--r-- | docs/internals/contributing/writing-code/unit-tests.txt | 2 | ||||
| -rw-r--r-- | docs/ref/migration-operations.txt | 17 | ||||
| -rw-r--r-- | docs/releases/1.7.txt | 8 | ||||
| -rw-r--r-- | tests/initial_sql_regress/tests.py | 1 | ||||
| -rw-r--r-- | tests/migrations/test_operations.py | 15 | ||||
| -rw-r--r-- | tests/requirements/base.txt | 1 |
11 files changed, 77 insertions, 38 deletions
diff --git a/django/core/management/sql.py b/django/core/management/sql.py index c789701c47..4832f27b07 100644 --- a/django/core/management/sql.py +++ b/django/core/management/sql.py @@ -155,6 +155,7 @@ def sql_all(app_config, style, connection): def _split_statements(content): + # Private API only called from code that emits a RemovedInDjango19Warning. comment_re = re.compile(r"^((?:'[^']*'|[^'])*?)--.*$") statements = [] statement = [] @@ -202,9 +203,7 @@ def custom_sql_for_model(model, style, connection): for sql_file in sql_files: if os.path.exists(sql_file): 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, - # so split into separate statements. - output.extend(_split_statements(fp.read())) + output.extend(connection.ops.prepare_sql_script(fp.read(), _allow_fallback=True)) return output diff --git a/django/db/backends/__init__.py b/django/db/backends/__init__.py index 10836f4651..4556fddd94 100644 --- a/django/db/backends/__init__.py +++ b/django/db/backends/__init__.py @@ -1,5 +1,6 @@ import datetime import time +import warnings try: 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.transaction import TransactionManagementError from django.db.utils import DatabaseError, DatabaseErrorWrapper, ProgrammingError +from django.utils.deprecation import RemovedInDjango19Warning from django.utils.functional import cached_property from django.utils import six from django.utils import timezone @@ -697,6 +699,10 @@ class BaseDatabaseFeatures(object): # Does 'a' LIKE 'A' match? 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): self.connection = connection @@ -972,6 +978,34 @@ class BaseDatabaseOperations(object): """ 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): """ Returns the value of a CLOB column, for backends that return a locator diff --git a/django/db/backends/postgresql_psycopg2/base.py b/django/db/backends/postgresql_psycopg2/base.py index cc44075ba7..91a183e8b5 100644 --- a/django/db/backends/postgresql_psycopg2/base.py +++ b/django/db/backends/postgresql_psycopg2/base.py @@ -59,6 +59,7 @@ class DatabaseFeatures(BaseDatabaseFeatures): nulls_order_largest = True closed_cursor_error_class = InterfaceError has_case_insensitive_like = False + requires_sqlparse_for_splitting = False class DatabaseWrapper(BaseDatabaseWrapper): diff --git a/django/db/backends/postgresql_psycopg2/operations.py b/django/db/backends/postgresql_psycopg2/operations.py index 9285e6eeca..b9d0231768 100644 --- a/django/db/backends/postgresql_psycopg2/operations.py +++ b/django/db/backends/postgresql_psycopg2/operations.py @@ -93,6 +93,9 @@ class DatabaseOperations(BaseDatabaseOperations): def no_limit_value(self): return None + def prepare_sql_script(self, sql, _allow_fallback=False): + return [sql] + def quote_name(self, name): if name.startswith('"') and name.endswith('"'): return name # Quoting once is enough. diff --git a/django/db/migrations/operations/special.py b/django/db/migrations/operations/special.py index 4f0c494f33..f87585f0db 100644 --- a/django/db/migrations/operations/special.py +++ b/django/db/migrations/operations/special.py @@ -1,4 +1,3 @@ -import re from .base import Operation @@ -43,20 +42,16 @@ class SeparateDatabaseAndState(Operation): class RunSQL(Operation): """ - Runs some raw SQL - a single statement by default, but it will attempt - to parse and split it into multiple statements if multiple=True. - - A reverse SQL statement may be provided. + Runs some raw SQL. A reverse SQL statement may be provided. 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. """ - 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.reverse_sql = reverse_sql self.state_operations = state_operations or [] - self.multiple = multiple @property def reversible(self): @@ -66,30 +61,15 @@ class RunSQL(Operation): for state_operation in self.state_operations: 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): - if self.multiple: - statements = self._split_sql(self.sql) - else: - statements = [self.sql] + statements = schema_editor.connection.ops.prepare_sql_script(self.sql) for statement in statements: schema_editor.execute(statement) def database_backwards(self, app_label, schema_editor, from_state, to_state): if self.reverse_sql is None: raise NotImplementedError("You cannot reverse this operation") - if self.multiple: - statements = self._split_sql(self.reverse_sql) - else: - statements = [self.reverse_sql] + statements = schema_editor.connection.ops.prepare_sql_script(self.reverse_sql) for statement in statements: schema_editor.execute(statement) diff --git a/docs/internals/contributing/writing-code/unit-tests.txt b/docs/internals/contributing/writing-code/unit-tests.txt index 3edc588c18..3571fb0c26 100644 --- a/docs/internals/contributing/writing-code/unit-tests.txt +++ b/docs/internals/contributing/writing-code/unit-tests.txt @@ -166,6 +166,7 @@ dependencies: * memcached_, plus a :ref:`supported Python binding <memcached>` * gettext_ (:ref:`gettext_on_windows`) * selenium_ +* sqlparse_ You can find these dependencies in `pip requirements files`_ inside the ``tests/requirements`` directory of the Django source tree and install them @@ -197,6 +198,7 @@ associated tests will be skipped. .. _memcached: http://memcached.org/ .. _gettext: http://www.gnu.org/software/gettext/manual/gettext.html .. _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 Code coverage diff --git a/docs/ref/migration-operations.txt b/docs/ref/migration-operations.txt index 2a5f46a93c..331ccd5c06 100644 --- a/docs/ref/migration-operations.txt +++ b/docs/ref/migration-operations.txt @@ -167,25 +167,23 @@ Changes a field's name (and, unless ``db_column`` is set, its column name). Special Operations ================== +.. _operation-run-sql: + 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 features of database backends that Django doesn't support directly, like partial indexes. -``sql``, and ``reverse_sql`` if provided, should be strings of SQL to run on the -database. They will be passed to the database as a single SQL statement unless -``multiple`` is set to ``True``, in which case they will be split into separate -statements manually by the operation before being passed through. - -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``. +``sql``, and ``reverse_sql`` if provided, should be strings of SQL to run on +the database. On most database backends (all but PostgreSQL), Django will +split the SQL into individual statements prior to executing them. This +requires installing the sqlparse_ Python library. 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 @@ -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 operation that adds that field and so will try to run it again). +.. _sqlparse: https://pypi.python.org/pypi/sqlparse .. _operation-run-python: diff --git a/docs/releases/1.7.txt b/docs/releases/1.7.txt index d0a0945d85..b620c1134b 100644 --- a/docs/releases/1.7.txt +++ b/docs/releases/1.7.txt @@ -636,6 +636,14 @@ Management Commands * :djadmin:`collectstatic` command with symlink option is now supported on 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 ^^^^^^ diff --git a/tests/initial_sql_regress/tests.py b/tests/initial_sql_regress/tests.py index 428d993667..ebbe36d35d 100644 --- a/tests/initial_sql_regress/tests.py +++ b/tests/initial_sql_regress/tests.py @@ -27,7 +27,6 @@ class InitialSQLTests(TestCase): """ connection = connections[DEFAULT_DB_ALIAS] custom_sql = custom_sql_for_model(Simple, no_style(), connection) - self.assertEqual(len(custom_sql), 9) with connection.cursor() as cursor: for sql in custom_sql: cursor.execute(sql) diff --git a/tests/migrations/test_operations.py b/tests/migrations/test_operations.py index e1b4682d58..d294bdcd26 100644 --- a/tests/migrations/test_operations.py +++ b/tests/migrations/test_operations.py @@ -1,5 +1,10 @@ import unittest +try: + import sqlparse +except ImportError: + sqlparse = None + from django.db import connection, migrations, models, router from django.db.migrations.migration import Migration from django.db.migrations.state import ProjectState @@ -640,6 +645,7 @@ class OperationTests(MigrationTestBase): operation.database_backwards("test_alinto", editor, new_state, project_state) 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): """ Tests the RunSQL operation. @@ -647,7 +653,10 @@ class OperationTests(MigrationTestBase): project_state = self.set_up_test_model("test_runsql") # Create the operation 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", state_operations=[migrations.CreateModel("SomethingElse", [("id", models.AutoField(primary_key=True))])], ) @@ -661,6 +670,10 @@ class OperationTests(MigrationTestBase): with connection.schema_editor() as editor: operation.database_forwards("test_runsql", editor, project_state, new_state) 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 self.assertTrue(operation.reversible) with connection.schema_editor() as editor: diff --git a/tests/requirements/base.txt b/tests/requirements/base.txt index 3d982bc00a..c3f77234fd 100644 --- a/tests/requirements/base.txt +++ b/tests/requirements/base.txt @@ -5,3 +5,4 @@ Pillow PyYAML pytz > dev selenium +sqlparse |