From a8b3f96f6acfa082f99166e0a1cfb4b0fbc0eace Mon Sep 17 00:00:00 2001 From: Simon Charette Date: Mon, 29 Apr 2019 22:49:45 -0400 Subject: [PATCH] Fixed #30408 -- Fixed crash when adding check constraints with LIKE operator on Oracle and PostgreSQL. The LIKE operator wildcard generated for contains, startswith, endswith and their case-insensitive variant lookups was conflicting with parameter interpolation on CREATE constraint statement execution. Ideally we'd delegate parameters interpolation in DDL statements on backends that support it but that would require backward incompatible changes to the Index and Constraint SQL generating methods. Thanks David Sanders for the report. --- django/db/backends/oracle/schema.py | 2 +- django/db/backends/postgresql/schema.py | 2 ++ docs/releases/2.2.1.txt | 4 +++ tests/migrations/test_operations.py | 43 +++++++++++++++++++++++++ 4 files changed, 50 insertions(+), 1 deletion(-) diff --git a/django/db/backends/oracle/schema.py b/django/db/backends/oracle/schema.py index 2d9c9699a0..3356788a82 100644 --- a/django/db/backends/oracle/schema.py +++ b/django/db/backends/oracle/schema.py @@ -23,7 +23,7 @@ class DatabaseSchemaEditor(BaseDatabaseSchemaEditor): if isinstance(value, (datetime.date, datetime.time, datetime.datetime)): return "'%s'" % value elif isinstance(value, str): - return "'%s'" % value.replace("\'", "\'\'") + return "'%s'" % value.replace("\'", "\'\'").replace('%', '%%') elif isinstance(value, (bytes, bytearray, memoryview)): return "'%s'" % value.hex() elif isinstance(value, bool): diff --git a/django/db/backends/postgresql/schema.py b/django/db/backends/postgresql/schema.py index 3319ee09a9..0738f009cd 100644 --- a/django/db/backends/postgresql/schema.py +++ b/django/db/backends/postgresql/schema.py @@ -24,6 +24,8 @@ class DatabaseSchemaEditor(BaseDatabaseSchemaEditor): sql_delete_procedure = 'DROP FUNCTION %(procedure)s(%(param_types)s)' def quote_value(self, value): + if isinstance(value, str): + value = value.replace('%', '%%') # getquoted() returns a quoted bytestring of the adapted value. return psycopg2.extensions.adapt(value).getquoted().decode() diff --git a/docs/releases/2.2.1.txt b/docs/releases/2.2.1.txt index 9f241b83e9..90c9a06e79 100644 --- a/docs/releases/2.2.1.txt +++ b/docs/releases/2.2.1.txt @@ -70,3 +70,7 @@ Bugfixes * Fixed a regression in Django 2.2 where changes were not reliably detected by auto-reloader when using ``StatReloader`` (:ticket:`30323`). + +* Fixed a migration crash on Oracle and PostgreSQL when adding a check + constraint with a ``contains``, ``startswith``, or ``endswith`` lookup (or + their case-insensitive variant) (:ticket:`30408`). diff --git a/tests/migrations/test_operations.py b/tests/migrations/test_operations.py index d5c8396596..520a2b2204 100644 --- a/tests/migrations/test_operations.py +++ b/tests/migrations/test_operations.py @@ -1855,6 +1855,49 @@ class OperationTests(OperationTestBase): self.assertEqual(definition[1], []) self.assertEqual(definition[2], {'model_name': "Pony", 'constraint': gt_constraint}) + @skipUnlessDBFeature('supports_table_check_constraints') + def test_add_constraint_percent_escaping(self): + app_label = 'add_constraint_string_quoting' + operations = [ + CreateModel( + 'Author', + fields=[ + ('id', models.AutoField(primary_key=True)), + ('name', models.CharField(max_length=100)), + ('rebate', models.CharField(max_length=100)), + ], + ), + ] + from_state = self.apply_operations(app_label, ProjectState(), operations) + # "%" generated in startswith lookup should be escaped in a way that is + # considered a leading wildcard. + check = models.Q(name__startswith='Albert') + constraint = models.CheckConstraint(check=check, name='name_constraint') + operation = migrations.AddConstraint('Author', constraint) + to_state = from_state.clone() + operation.state_forwards(app_label, to_state) + with connection.schema_editor() as editor: + operation.database_forwards(app_label, editor, from_state, to_state) + Author = to_state.apps.get_model(app_label, 'Author') + with self.assertRaises(IntegrityError), transaction.atomic(): + Author.objects.create(name='Artur') + # Literal "%" should be escaped in a way that is not a considered a + # wildcard. + check = models.Q(rebate__endswith='%') + constraint = models.CheckConstraint(check=check, name='rebate_constraint') + operation = migrations.AddConstraint('Author', constraint) + from_state = to_state + to_state = from_state.clone() + operation.state_forwards(app_label, to_state) + Author = to_state.apps.get_model(app_label, 'Author') + with connection.schema_editor() as editor: + operation.database_forwards(app_label, editor, from_state, to_state) + Author = to_state.apps.get_model(app_label, 'Author') + with self.assertRaises(IntegrityError), transaction.atomic(): + Author.objects.create(name='Albert', rebate='10$') + author = Author.objects.create(name='Albert', rebate='10%') + self.assertEqual(Author.objects.get(), author) + @skipUnlessDBFeature('supports_table_check_constraints') def test_remove_constraint(self): project_state = self.set_up_test_model("test_removeconstraint", constraints=[