Opened 4 months ago

Closed 4 months ago

Last modified 4 months ago

#35625 closed Bug (fixed)

ProgrammingError with Postgres backend when setting db_default and constraint using the LIKE operator

Reported by: Julien Chaumont Owned by: Simon Charette
Component: Migrations Version: 5.0
Severity: Release blocker Keywords: db_default, postgres
Cc: 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

A ProgrammingError is raised when declaring a model with a field using db_default and a constraint such as field__startswith when the database backend is postgres.

Example:

# models.py
class MyFoo(models.Model):
    name = models.CharField(default="", db_default="", max_length=255)

    class Meta:
        constraints = [
            models.CheckConstraint(
                check=models.Q(name__startswith="foo"),
                name="name_startswith_foo"
            )
        ]

# migrations/0002_my_migration.py
class Migration(migrations.Migration):

    dependencies = [
        ('my_app', '0001_initial'),
    ]

    operations = [
        migrations.CreateModel(
            name='MyFoo',
            fields=[
                ('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
                ('name', models.CharField(default='', db_default='', max_length=255)),
            ],
            options={
                "constraints": [
                    models.CheckConstraint(check=models.Q(('name__startswith', 'foo')), name='name_startswith_foo')
                ]
            }
        ),
    ]

Then if you run sqlmigrate:

$ python manage.py sqlmigrate my_app 0001
Traceback (most recent call last):
  File "/app/manage.py", line 23, in <module>
    main()
  File "/app/manage.py", line 19, in main
    execute_from_command_line(sys.argv)
  File "/venv/lib/python3.12/site-packages/django/core/management/__init__.py", line 442, in execute_from_command_line
    utility.execute()
  File "/venv/lib/python3.12/site-packages/django/core/management/__init__.py", line 436, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/venv/lib/python3.12/site-packages/django/core/management/base.py", line 413, in run_from_argv
    self.execute(*args, **cmd_options)
  File "/venv/lib/python3.12/site-packages/django/core/management/commands/sqlmigrate.py", line 38, in execute
    return super().execute(*args, **options)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/venv/lib/python3.12/site-packages/django/core/management/base.py", line 459, in execute
    output = self.handle(*args, **options)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/venv/lib/python3.12/site-packages/django/core/management/commands/sqlmigrate.py", line 80, in handle
    sql_statements = loader.collect_sql(plan)
                     ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/venv/lib/python3.12/site-packages/django/db/migrations/loader.py", line 381, in collect_sql
    state = migration.apply(state, schema_editor, collect_sql=True)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/venv/lib/python3.12/site-packages/django/db/migrations/migration.py", line 132, in apply
    operation.database_forwards(
  File "/venv/lib/python3.12/site-packages/django/db/migrations/operations/models.py", line 96, in database_forwards
    schema_editor.create_model(model)
  File "/venv/lib/python3.12/site-packages/django/db/backends/base/schema.py", line 492, in create_model
    self.execute(sql, params or None)
  File "/venv/lib/python3.12/site-packages/django/db/backends/postgresql/schema.py", line 46, in execute
    sql = self.connection.ops.compose_sql(str(sql), params)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/venv/lib/python3.12/site-packages/django/db/backends/postgresql/operations.py", line 195, in compose_sql
    return mogrify(sql, params, self.connection)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/venv/lib/python3.12/site-packages/django/db/backends/postgresql/psycopg_any.py", line 22, in mogrify
    return ClientCursor(cursor.connection).mogrify(sql, params)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/venv/lib/python3.12/site-packages/psycopg/client_cursor.py", line 45, in mogrify
    pgq = self._convert_query(query, params)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/venv/lib/python3.12/site-packages/psycopg/_cursor_base.py", line 455, in _convert_query
    pgq.convert(query, params)
  File "/venv/lib/python3.12/site-packages/psycopg/_queries.py", line 264, in convert
    (self.template, self._order, self._parts) = f(bquery, self._encoding)
                                                ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/venv/lib/python3.12/site-packages/psycopg/_queries.py", line 298, in _query2pg_client_nocache
    parts = _split_query(query, encoding, collapse_double_percent=False)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/venv/lib/python3.12/site-packages/psycopg/_queries.py", line 398, in _split_query
    raise e.ProgrammingError(
psycopg.ProgrammingError: only '%s', '%b', '%t' are allowed as placeholders, got '%''

With pdb, we can see what the SQL query looks like at this stage:

(Pdb) query
b'CREATE TABLE "my_app_myfoo" ("id" bigint NOT NULL PRIMARY KEY GENERATED BY DEFAULT AS IDENTITY, "name" varchar(255) DEFAULT %s NOT NULL, CONSTRAINT "name_startswit_foo" CHECK ("name"::text LIKE \'foo%\'))'

A similar error happens when using psycopg2 instead of psycopg3. However, everything is fine when using the sqlite backend.

ℹ️ This migration is not the one auto-generated by django with the makemigrations command. It is however a valid migration that matches the model definition. This is also the SQL generated when setting up the test database with pytest when running with --no-migrations.

$ pytest --create-db --no-migrations
/venv/lib/python3.12/site-packages/pytest_django/plugin.py:532: in _django_db_marker
    request.getfixturevalue("_django_db_helper")
/venv/lib/python3.12/site-packages/pytest_django/fixtures.py:139: in django_db_setup
    db_cfg = setup_databases(
/venv/lib/python3.12/site-packages/django/test/utils.py:203: in setup_databases
    connection.creation.create_test_db(
/venv/lib/python3.12/site-packages/django/db/backends/base/creation.py:78: in create_test_db
    call_command(
/venv/lib/python3.12/site-packages/django/core/management/__init__.py:194: in call_command
    return command.execute(*args, **defaults)
/venv/lib/python3.12/site-packages/django/core/management/base.py:459: in execute
    output = self.handle(*args, **options)
/venv/lib/python3.12/site-packages/pytest_django/fixtures.py:304: in handle
    return super().handle(*args, **kwargs)
/venv/lib/python3.12/site-packages/django/core/management/base.py:107: in wrapper
    res = handle_func(*args, **kwargs)
/venv/lib/python3.12/site-packages/django/core/management/commands/migrate.py:321: in handle
    self.sync_apps(connection, executor.loader.unmigrated_apps)
/venv/lib/python3.12/site-packages/django/core/management/commands/migrate.py:483: in sync_apps
    editor.create_model(model)
/venv/lib/python3.12/site-packages/django/db/backends/base/schema.py:492: in create_model
    self.execute(sql, params or None)
/venv/lib/python3.12/site-packages/django/db/backends/postgresql/schema.py:46: in execute
    sql = self.connection.ops.compose_sql(str(sql), params)
/venv/lib/python3.12/site-packages/django/db/backends/postgresql/operations.py:195: in compose_sql
    return mogrify(sql, params, self.connection)
/venv/lib/python3.12/site-packages/django/db/backends/postgresql/psycopg_any.py:22: in mogrify
    return ClientCursor(cursor.connection).mogrify(sql, params)
/venv/lib/python3.12/site-packages/psycopg/client_cursor.py:45: in mogrify
    pgq = self._convert_query(query, params)
/venv/lib/python3.12/site-packages/psycopg/_cursor_base.py:455: in _convert_query
    pgq.convert(query, params)
/venv/lib/python3.12/site-packages/psycopg/_queries.py:264: in convert
    (self.template, self._order, self._parts) = f(bquery, self._encoding)
/venv/lib/python3.12/site-packages/psycopg/_queries.py:298: in _query2pg_client_nocache
    parts = _split_query(query, encoding, collapse_double_percent=False)
psycopg.ProgrammingError: only '%s', '%b', '%t' are allowed as placeholders, got '%''

Change History (7)

comment:1 by Simon Charette, 4 months ago

Owner: set to Simon Charette
Status: newassigned
Triage Stage: UnreviewedAccepted

Managed to reproduce. The issue is in the same vein as #35336 and the long tail of problems related to improper escaping handling in the schema editor (#30408, #34553, #32369).

The big design problem here is that the Constraint.(constraint|create|delete)_sql interface returns sql: str instead of (sql: str, params: tuple) to the schema editor which has no other choice not to use backend bindings (passing (sql, None) to cursor.execute) but it decides to do so when requires_literal_defaults = False (PostgresSQL, MySQL) for DEFAULT. In other words, unless we change Constraint and Index to return un-parametrized SQL we cannot use parametrized SQL when performing any operation that involve them.

I think we should mark this one as a release blocker because db_default was added in 5.0 and the latter still receives patches for bugs in new features until 5.1 is released?

I'll try to work on a non-invasive patch that can be backported and likely create another issue to discuss switching the whole DDL generation machinery to provide (sql: str, params: tuple) and let the schema editor decide how it wants to deal with parametrization (use it on Postgres + MySQL) and rely on quote_value otherwise through a single supports_parametrized_ddl feature flag.

Version 0, edited 4 months ago by Simon Charette (next)

comment:2 by Simon Charette, 4 months ago

Has patch: set
Severity: NormalRelease blocker

comment:3 by Mariusz Felisiak, 4 months ago

Triage Stage: AcceptedReady for checkin

comment:4 by Sarah Boyce <42296566+sarahboyce@…>, 4 months ago

Resolution: fixed
Status: assignedclosed

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.

comment:5 by Sarah Boyce <42296566+sarahboyce@…>, 4 months ago

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.

comment:6 by Sarah Boyce <42296566+sarahboyce@…>, 4 months ago

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.

comment:7 by Sarah Boyce <42296566+sarahboyce@…>, 4 months ago

In ca9880b9:

[5.0.x] Refs #35625 -- Corrected CheckConstraint argument name in migrations.test_operations.

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