[Django] #30408: CheckConstraint with lookup using LIKE & % and PostgreSQL results in exception
Django
exception
-------------------------------------+-------------------------------------
Reporter: David | Owner: nobody
Sanders |
Type: Bug | Status: new
Component: Database | Version: master
layer (models, ORM) |
Severity: Normal | Keywords:
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/30408>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Reporter: David Sanders | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 2.2
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: Ian Foote (added)
* version: master => 2.2
* severity: Normal => Release blocker
* stage: Unreviewed => Accepted
Comment:
Reproduced at efeceba589974b95b35b2e25df86498c96315518.
--
Ticket URL: <https://code.djangoproject.com/ticket/30408#comment:1>
Django
-------------------------------------+-------------------------------------
Reporter: David Sanders | Owner: nobody
Component: Database layer | Version: 2.2
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* Attachment "30408.diff" added.
Django
-------------------------------------+-------------------------------------
| Charette
Type: Bug | Status: assigned
Component: Database layer | Version: 2.2
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => assigned
* owner: nobody => Simon Charette
Comment:
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 [https://github.com/django/django/compare/master...charettes
:constraint-index-sql ungoing work to make this happen].
I'll tentatively assign to myself to try to polish the solution suggested
above.
--
Ticket URL: <https://code.djangoproject.com/ticket/30408#comment:2>
Django
-------------------------------------+-------------------------------------
Reporter: David Sanders | Owner: Simon
| Charette
Type: Bug | Status: assigned
Component: Database layer | Version: 2.2
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* has_patch: 0 => 1
Comment:
I continued efforts to get the `Index` and `Constraint`'s `sql` methods
return `(sql, params)` tuples but I'm having a hard time
[https://github.com/django/django/compare/master...charettes:constraint-
index-sql 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 [https://github.com/django/django/pull/11306 a PR] that performs
the `%` escaping in `quote_value` and should address the immediate issue.
--
Ticket URL: <https://code.djangoproject.com/ticket/30408#comment:3>
Django
-------------------------------------+-------------------------------------
Reporter: David Sanders | Owner: Simon
| Charette
Type: Bug | Status: assigned
Component: Database layer | Version: 2.2
(models, ORM) |
Severity: Release blocker | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/30408#comment:4>
Django
-------------------------------------+-------------------------------------
Reporter: David Sanders | Owner: Simon
| Charette
Component: Database layer | Version: 2.2
(models, ORM) |
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"a8b3f96f6acfa082f99166e0a1cfb4b0fbc0eace" a8b3f96f]:
{{{
#!CommitTicketReference repository=""
revision="a8b3f96f6acfa082f99166e0a1cfb4b0fbc0eace"
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.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30408#comment:5>
Django
-------------------------------------+-------------------------------------
Reporter: David Sanders | Owner: Simon
| Charette
Type: Bug | Status: closed
Component: Database layer | Version: 2.2
(models, ORM) |
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"f36239fa190c77883d4c09ad18da63278c1a6cf4" f36239f]:
{{{
#!CommitTicketReference repository=""
revision="f36239fa190c77883d4c09ad18da63278c1a6cf4"
[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
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30408#comment:6>
Django
-------------------------------------+-------------------------------------
Reporter: David Sanders | Owner: Simon
| Charette
Type: Bug | Status: closed
Component: Database layer | Version: 2.2
(models, ORM) |
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"ffff17d4b0117cce59f65c9f56fa164694eafd23" ffff17d4]:
{{{
#!CommitTicketReference repository=""
revision="ffff17d4b0117cce59f65c9f56fa164694eafd23"
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.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30408#comment:7>