Opened 3 years ago

Closed 3 years ago

Last modified 12 months ago

#32369 closed Bug (fixed)

CheckConstraints with pattern lookups and expressions as right-hand sides crash on Oracle, MySQL, and PostgreSQL.

Reported by: Jaroslav Semančík Owned by: Simon Charette
Component: Database layer (models, ORM) Version: 3.1
Severity: Normal 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

After fixing of #30408 a similar bug persists in case of indirect right-hand side, e.g. when comparing two model fields.

Consider model:

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

    class Meta:
        constraints = (
            models.CheckConstraint(
                check=models.Q(foo__startswith=models.F('bar')),
                name='check_foo_starts_with_bar',
            ),
        )

After making migration and running sqlmigrate with PostgreSQL backend we get TypeError: not enough arguments for format string.

Traceback:

Traceback (most recent call last):
  File "/snap/pycharm-professional/228/plugins/python/helpers/pycharm/django_manage.py", line 52, in <module>
    run_command()
  File "/snap/pycharm-professional/228/plugins/python/helpers/pycharm/django_manage.py", line 46, in run_command
    run_module(manage_file, None, '__main__', True)
  File "/usr/lib/python3.9/runpy.py", line 210, in run_module
    return _run_module_code(code, init_globals, run_name, mod_spec)
  File "/usr/lib/python3.9/runpy.py", line 97, in _run_module_code
    _run_code(code, mod_globals, init_globals,
  File "/usr/lib/python3.9/runpy.py", line 87, in _run_code
    exec(code, run_globals)
  File "/home/giro/Projects/djangostartswith/manage.py", line 22, in <module>
    main()
  File "/home/giro/Projects/djangostartswith/manage.py", line 18, in main
    execute_from_command_line(sys.argv)
  File "/home/giro/src/django/core/management/__init__.py", line 401, in execute_from_command_line
    utility.execute()
  File "/home/giro/src/django/core/management/__init__.py", line 395, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/home/giro/src/django/core/management/base.py", line 330, in run_from_argv
    self.execute(*args, **cmd_options)
  File "/home/giro/src/django/core/management/commands/sqlmigrate.py", line 29, in execute
    return super().execute(*args, **options)
  File "/home/giro/src/django/core/management/base.py", line 371, in execute
    output = self.handle(*args, **options)
  File "/home/giro/src/django/core/management/commands/sqlmigrate.py", line 65, in handle
    sql_statements = loader.collect_sql(plan)
  File "/home/giro/src/django/db/migrations/loader.py", line 345, in collect_sql
    state = migration.apply(state, schema_editor, collect_sql=True)
  File "/home/giro/src/django/db/migrations/migration.py", line 124, in apply
    operation.database_forwards(self.app_label, schema_editor, old_state, project_state)
  File "/home/giro/src/django/db/migrations/operations/models.py", line 808, in database_forwards
    schema_editor.add_constraint(model, self.constraint)
  File "/home/giro/src/django/db/backends/base/schema.py", line 362, in add_constraint
    self.execute(sql)
  File "/home/giro/src/django/db/backends/base/schema.py", line 137, in execute
    self.collected_sql.append((sql % tuple(map(self.quote_value, params))) + ending)
TypeError: not enough arguments for format string

The cause is again an unescaped % in the generated SQL.

Change History (8)

comment:1 by Mariusz Felisiak, 3 years ago

Cc: Ian Foote added
Summary: CheckConstraint with pattern lookup & another field crashes on PostgreSQLCheckConstraints with pattern lookups and expressions as right-hand sides crash on Oracle and PostgreSQL.
Triage Stage: UnreviewedAccepted

Thanks for the report.

Reproduced at bbd18943c6c00fb1386ecaaf6771a54f780ebf62.

comment:2 by Simon Charette, 3 years ago

Owner: changed from nobody to Simon Charette
Status: newassigned

comment:3 by Simon Charette, 3 years ago

Has patch: set

comment:4 by Mariusz Felisiak, 3 years ago

Summary: CheckConstraints with pattern lookups and expressions as right-hand sides crash on Oracle and PostgreSQL.CheckConstraints with pattern lookups and expressions as right-hand sides crash on Oracle, MySQL, and PostgreSQL.
Triage Stage: AcceptedReady for checkin

comment:5 by Mariusz Felisiak <felisiak.mariusz@…>, 3 years ago

Resolution: fixed
Status: assignedclosed

In 42e8cf47:

Fixed #32369 -- Fixed adding check constraints with pattern lookups and expressions as rhs.

This disables interpolation of constraint creation statements. Since
Constraint.create_sql interpolates its parameters instead of deferring
this responsibility to the backend connection it must disable
connection level parameters interpolation.

comment:6 by Mariusz Felisiak <felisiak.mariusz@…>, 3 years ago

In 9607e3a0:

[3.2.x] Fixed #32369 -- Fixed adding check constraints with pattern lookups and expressions as rhs.

This disables interpolation of constraint creation statements. Since
Constraint.create_sql interpolates its parameters instead of deferring
this responsibility to the backend connection it must disable
connection level parameters interpolation.

Backport of 42e8cf47c7ee2db238bf91197ea398126c546741 from master

comment:7 by Mariusz Felisiak <felisiak.mariusz@…>, 12 months ago

In e0f8104a:

Refs #34553 -- Split constraint escaping test in subtests.

This ensures that constraint violations are tested in isolation from
each other as an IntegrityError only ensures a least one constraint is
violated.

For example, the assertion added in 42e8cf4 break both the
name_constraint_rhs and the rebate_constraint constraints and thus
doesn't constitute a proper regression test. Refs #32369.

comment:8 by Mariusz Felisiak <felisiak.mariusz@…>, 12 months ago

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.

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