#35336: Adding GeneratedField fails with ProgrammingError when using When on
CharField
-------------------------------------+-------------------------------------
Reporter: Adrian Garcia | Owner: Simon
| Charette
Type: Bug | Status: assigned
Component: Database layer | Version: 5.0
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: postgres, | Triage Stage: Accepted
generatedfield, contains |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Simon Charette):
* has_patch: 0 => 1
* needs_better_patch: 0 => 1
Comment:
> Out of curiosity, why does this issue only present itself when adding a
field? Does Django handle SQL generation differently for table creation vs
field addition?
I'm still laying down the details
[https://github.com/django/django/pull/18027 in this MR] but the short
answer is yes.
The DBAPI `Cursor.execute` method can be used in two ways when dealing
with parameters. Either `(sql: str, params: [])` which will delegate
parametrization to the library or `(sql: str, params: None)` which won't
attempt to parametrize the SQL. This is an important difference and
requires special care when dealing with `%` literals in parameters as it's
the symbol use for parametrization placeholders by most libraries.
Well some backend simply don't support parametrization in some DDL
statements such as `GENERATED` columns declaration, in other words you
can't do
{{{#!python
cursor.execute(
"ALTER TABLE invoice ADD COLUMN discount_display text GENERATED ALWAYS
discount || %s;",
["%"]
)
}}}
So you must do
{{{#!python
cursor.execute(
"ALTER TABLE invoice ADD COLUMN discount_display text GENERATED ALWAYS
AS discount || '%';", None
)
}}}
Well in the case of `SchemaEditor.create_model` we liberally do one or the
other and in your case, because you don't have any other field with a
`db_default` assuming your use Postgres, it does the right thing.
In the case of `SchemaEditor.add_field` however we always do `params:
list`, which forces parametrization, and thus
{{{#!python
cursor.execute(
"ALTER TABLE testmodel ADD COLUMN contains_heck text GENERATED ALWAYS
AS UPPER(description) LIKE '%HECK%';", []
)
}}}
Crashes as there is an attempt to interpolate the two `%` while `params`
is empty.
The schema migration has historically use parametrizable DDL in the past
when possible but this has become increasingly complex as more and more
features were added (indexes, constraints, `db_default`, `GeneratedField`)
to a point where I don't think we can continue doing so but the tricky
part is that we can't make such a change in a bug fix release without
risking to introduce further problems. This is made even worst by the fact
that objects as `Index` and `Constraint` subclasses return already
interpolated SQL (their `as_sql` methods return `"CHECK UPPER(description)
LIKE '%HECK%'"` instead of `("CHECK UPPER(description) LIKE %s",
"%HECK%")` so even if we wanted to use it entirely to the backend until
the last minute by using `quote_value` ourselves we can't as by that point
we have a mixture of interpolated and non-interpolated SQL.
--
Ticket URL: <https://code.djangoproject.com/ticket/35336#comment:3>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/0107018e85bd49fd-1ec7c223-0d2b-42a0-b93a-b156e26d2245-000000%40eu-central-1.amazonses.com.
