CheckConstraint with lookup using LIKE & % crash on Oracle and PostgreSQL.

[David Sanders]

Given the following model:

class Foo(models.Model):
    bar = models.CharField(max_length=255)

    class Meta:
        constraints = (
            models.CheckConstraint(
                check=models.Q(bar__startswith='BAR'),
                name='check_bar_starts_with_BAR',
            ),
        )

Running migrate with PostgreSQL will result in an exception:

davids ~/projects/test_startswith_constraint $ ./manage.py migrate
Operations to perform:
  Apply all migrations: admin, auth, contenttypes, sample, sessions
Running migrations:
  Applying contenttypes.0001_initial... OK
  Applying auth.0001_initial... OK
  Applying admin.0001_initial... OK
  Applying admin.0002_logentry_remove_auto_add... OK
  Applying admin.0003_logentry_add_action_flag_choices... OK
  Applying contenttypes.0002_remove_content_type_name... OK
  Applying auth.0002_alter_permission_name_max_length... OK
  Applying auth.0003_alter_user_email_max_length... OK
  Applying auth.0004_alter_user_username_opts... OK
  Applying auth.0005_alter_user_last_login_null... OK
  Applying auth.0006_require_contenttypes_0002... OK
  Applying auth.0007_alter_validators_add_error_messages... OK
  Applying auth.0008_alter_user_username_max_length... OK
  Applying auth.0009_alter_user_last_name_max_length... OK
  Applying auth.0010_alter_group_name_max_length... OK
  Applying auth.0011_update_proxy_permissions... OK
  Applying sample.0001_initial...Traceback (most recent call last):
  File "./manage.py", line 21, in <module>
    main()
  File "./manage.py", line 17, in main
    execute_from_command_line(sys.argv)
  File "/Users/davids/src/django/django/core/management/__init__.py", line 381, in execute_from_command_line
    utility.execute()
  File "/Users/davids/src/django/django/core/management/__init__.py", line 375, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/Users/davids/src/django/django/core/management/base.py", line 323, in run_from_argv
    self.execute(*args, **cmd_options)
  File "/Users/davids/src/django/django/core/management/base.py", line 364, in execute
    output = self.handle(*args, **options)
  File "/Users/davids/src/django/django/core/management/base.py", line 83, in wrapped
    res = handle_func(*args, **kwargs)
  File "/Users/davids/src/django/django/core/management/commands/migrate.py", line 233, in handle
    fake_initial=fake_initial,
  File "/Users/davids/src/django/django/db/migrations/executor.py", line 117, in migrate
    state = self._migrate_all_forwards(state, plan, full_plan, fake=fake, fake_initial=fake_initial)
  File "/Users/davids/src/django/django/db/migrations/executor.py", line 147, in _migrate_all_forwards
    state = self.apply_migration(state, migration, fake=fake, fake_initial=fake_initial)
  File "/Users/davids/src/django/django/db/migrations/executor.py", line 245, in apply_migration
    state = migration.apply(state, schema_editor)
  File "/Users/davids/src/django/django/db/migrations/migration.py", line 124, in apply
    operation.database_forwards(self.app_label, schema_editor, old_state, project_state)
  File "/Users/davids/src/django/django/db/migrations/operations/models.py", line 827, in database_forwards
    schema_editor.add_constraint(model, self.constraint)
  File "/Users/davids/src/django/django/db/backends/base/schema.py", line 346, in add_constraint
    self.execute(sql)
  File "/Users/davids/src/django/django/db/backends/base/schema.py", line 138, in execute
    cursor.execute(sql, params)
  File "/Users/davids/src/django/django/db/backends/utils.py", line 99, in execute
    return super().execute(sql, params)
  File "/Users/davids/src/django/django/db/backends/utils.py", line 67, in execute
    return self._execute_with_wrappers(sql, params, many=False, executor=self._execute)
  File "/Users/davids/src/django/django/db/backends/utils.py", line 76, in _execute_with_wrappers
    return executor(sql, params, many, context)
  File "/Users/davids/src/django/django/db/backends/utils.py", line 84, in _execute
    return self.cursor.execute(sql, params)
IndexError: tuple index out of range

This is due to the SQL being passed to execute() has an unescaped %:

> /Users/davids/src/django/django/db/backends/utils.py(85)_execute()
     84                 import ipdb; ipdb.set_trace()
---> 85                 return self.cursor.execute(sql, params)
     86

ipdb> sql
'ALTER TABLE "sample_foo" ADD CONSTRAINT "check_bar_starts_with_BAR" CHECK ("bar"::text LIKE \'BAR%\')'

Note that this runs fine with SQLite but is problematic for PostgreSQL.

[Mariusz Felisiak]

Reproduced at efeceba589974b95b35b2e25df86498c96315518.

[Simon Charette]

FWIW it's the same class of issue as #30258 which will probably better be fixed by making create_sql/constraint_sql methods return (sql, params) tuples. See https://code.djangoproject.com/ticket/30258#comment:3 and ​ungoing work to make this happen.

I'll tentatively assign to myself to try to polish the solution suggested above.

[Simon Charette]

I continued efforts to get the Index and Constraint's sql methods return (sql, params) tuples but I'm having a hard time ​making the changes backward compatible.

It looks like we'll have to keep playing the whac-a-mole game in 2.2 by extending backends schema editor's quote_value support for more input. One good side effect of these changes is that it will extend sqlmigrate coverage which also relies on appropriate output from this method.

Here's ​a PR that performs the % escaping in quote_value and should address the immediate issue.

[Mariusz Felisiak <felisiak.mariusz@…>]

In a8b3f96f:

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.

[Mariusz Felisiak <felisiak.mariusz@…>]

In f36239f:

[2.2.x] 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.

Backport of a8b3f96f6acfa082f99166e0a1cfb4b0fbc0eace from master

[Mariusz Felisiak <felisiak.mariusz@…>]

In ffff17d4:

Fixed #34553 -- Fixed improper % escaping of literal in constraints.

Proper escaping of % in string literals used when defining constaints
was attempted (a8b3f96f6) by overriding quote_value of Postgres and
Oracle schema editor. The same approach was used when adding support for
constraints to the MySQL/MariaDB backend (1fc2c70).

Later on it was discovered that this approach was not appropriate and
that a preferable one was to pass params=None when executing the
constraint creation DDL to avoid any form of interpolation in the first
place (42e8cf47).

When the second patch was applied the corrective of the first were not
removed which caused % literals to be unnecessary doubled. This flew
under the radar because the existings test were crafted in a way that
consecutive %% didn't catch regressions.

This commit introduces an extra test for exact lookups which
highlights more adequately % doubling problems but also adjust a
previous endswith test to cover % doubling problems (%\% -> %%\%%).

Thanks Thomas Kolar for the report.

Refs #32369, #30408, #30593.

[Sarah Boyce <42296566+sarahboyce@…>]

In f359990e:

Fixed #35625 -- Fixed a crash when adding a field with db_default and check constraint.

This is the exact same issue as refs #30408 but for creating a model with a
constraint containing % escapes instead of column addition. All of these issues
stem from a lack of SQL and parameters separation from the BaseConstraint DDL
generating methods preventing them from being mixed with other parts of the
schema alteration logic that do make use of parametrization on some backends
(e.g. Postgres, MySQL for DEFAULT).

Prior to the addition of Field.db_default and GeneratedField in 5.0
parametrization of DDL was never exercised on model creation so this is
effectively a bug with db_default as the GeneratedField case was addressed by
refs #35336.

Thanks Julien Chaumont for the report and Mariusz Felisiak for the review.

[Sarah Boyce <42296566+sarahboyce@…>]

In d8116bf7:

[5.1.x] Fixed #35625 -- Fixed a crash when adding a field with db_default and check constraint.

This is the exact same issue as refs #30408 but for creating a model with a
constraint containing % escapes instead of column addition. All of these issues
stem from a lack of SQL and parameters separation from the BaseConstraint DDL
generating methods preventing them from being mixed with other parts of the
schema alteration logic that do make use of parametrization on some backends
(e.g. Postgres, MySQL for DEFAULT).

Prior to the addition of Field.db_default and GeneratedField in 5.0
parametrization of DDL was never exercised on model creation so this is
effectively a bug with db_default as the GeneratedField case was addressed by
refs #35336.

Thanks Julien Chaumont for the report and Mariusz Felisiak for the review.

Backport of f359990e4909db8722820849d61a6f5724338723 from main.

[Sarah Boyce <42296566+sarahboyce@…>]

In 41d8ef18:

[5.0.x] Fixed #35625 -- Fixed a crash when adding a field with db_default and check constraint.

This is the exact same issue as refs #30408 but for creating a model with a
constraint containing % escapes instead of column addition. All of these issues
stem from a lack of SQL and parameters separation from the BaseConstraint DDL
generating methods preventing them from being mixed with other parts of the
schema alteration logic that do make use of parametrization on some backends
(e.g. Postgres, MySQL for DEFAULT).

Prior to the addition of Field.db_default and GeneratedField in 5.0
parametrization of DDL was never exercised on model creation so this is
effectively a bug with db_default as the GeneratedField case was addressed by
refs #35336.

Thanks Julien Chaumont for the report and Mariusz Felisiak for the review.

Backport of f359990e4909db8722820849d61a6f5724338723 from main.