Opened 2 months ago

Closed 8 weeks ago

Last modified 8 weeks ago

#30408 closed Bug (fixed)

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

Reported by: David Sanders Owned by: Simon Charette
Component: Database layer (models, ORM) Version: 2.2
Severity: Release blocker Keywords:
Cc: Ian Foote Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

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.

Attachments (1)

30408.diff (1.5 KB) - added by felixxm 2 months ago.

Download all attachments as: .zip

Change History (7)

comment:1 Changed 2 months ago by felixxm

Cc: Ian Foote added
Severity: NormalRelease blocker
Summary: CheckConstraint with lookup using LIKE & % and PostgreSQL results in exceptionCheckConstraint with lookup using LIKE & % crash on PostgreSQL.
Triage Stage: UnreviewedAccepted
Version: master2.2

Changed 2 months ago by felixxm

Attachment: 30408.diff added

comment:2 Changed 2 months ago by Simon Charette

Owner: changed from nobody to Simon Charette
Status: newassigned

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.

comment:3 Changed 8 weeks ago by Simon Charette

Has patch: set

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.

comment:4 Changed 8 weeks ago by felixxm

Summary: CheckConstraint with lookup using LIKE & % crash on PostgreSQL.CheckConstraint with lookup using LIKE & % crash on Oracle and PostgreSQL.
Triage Stage: AcceptedReady for checkin

comment:5 Changed 8 weeks ago by Mariusz Felisiak <felisiak.mariusz@…>

Resolution: fixed
Status: assignedclosed

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.

comment:6 Changed 8 weeks ago by 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

Note: See TracTickets for help on using tickets.
Back to Top