Opened 21 hours ago
Last modified 9 hours ago
#35969 new Bug
Changing output_field for GeneratedField leads to ProgrammingError with Postgres 16.5+
Reported by: | Ryan Schave | Owned by: | |
---|---|---|---|
Component: | Migrations | Version: | 5.1 |
Severity: | Normal | Keywords: | |
Cc: | Lily Foote, Mariusz Felisiak, Simon Charette | Triage Stage: | Accepted |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Consider the following model and assume the initial migration has been applied:
class Order(models.Model): order_no = models.IntegerField() item_no = models.CharField(max_length=25) qty = models.IntegerField() cost = models.DecimalField(max_digits=10, decimal_places=2) total_cost = models.GeneratedField( expression=F("cost") * F("qty"), output_field=models.BigIntegerField(), db_persist=True, )
During a code review we determined the output field should be a Decimal field and the field was modified as follows:
total_cost = models.GeneratedField( expression=F("cost") * F("qty"), output_field=models.DecimalField(decimal_places=2, max_digits=16), db_persist=True, )
And a new migration was generated:
migrations.AlterField( model_name='order', name='total_cost', field=models.GeneratedField(db_persist=True, expression=django.db.models.expressions.CombinedExpression(models.F('cost'), '*', models.F('qty')), output_field=models.DecimalField(decimal_places=2, max_digits=16)), ),
In Postgres 16.4 and earlier, this migration is applied without error. (I'm aware that the value of the total_cost field is not recomputed for existing records when this migration is applied.).
Starting with Postgres 16.5 and up, this migration fails with the following error:
psycopg2.errors.InvalidColumnDefinition: cannot specify USING when altering type of generated column DETAIL: Column "total_cost" is a generated column. ... django.db.utils.ProgrammingError: cannot specify USING when altering type of generated column
This appears to be a result of the following change in Postgres 16.5 (release notes):
Disallow a
USING
clause when altering the type of a generated column (Peter Eisentraut) [§](https://postgr.es/c/5867ee005)
A generated column already has an expression specifying the column contents, so includingUSING
doesn't make sense.
The Django documentation for GeneratedField makes it clear that
There are many database-specific restrictions on generated fields that Django doesn’t validate and the database may raise an error
For this reason, I almost didn't open a ticket. However, there is logic in db/backends/base/schema.py that checks if the expression of a GeneratedField changed. Consider this migration (which changes the expression from multiplication to addition):
migrations.AlterField( model_name='order', name='total_cost', field=models.GeneratedField(db_persist=True, expression=django.db.models.expressions.CombinedExpression(models.F('cost'), '+', models.F('qty')), output_field=models.BigIntegerField()), ),
Attempting to apply this migration will raise the following error (even in Postgres 16.4):
ValueError: Modifying GeneratedFields is not supported - the field sales.Order.total_cost must be removed and re-added with the new definition.
This error is more helpful. It explains the problem better and even suggests a workaround.
Should we throw a similar error if the output_field of a GeneratedField is changed? Or add a tip to the documentation?
The above was tested with:
Django version 5.1.3
psycopg2 version 2.9.10
Postgres versions: 16.4, 16.5, 16.6, 17.0, and 17.2
Change History (6)
comment:1 by , 17 hours ago
Cc: | added |
---|---|
Component: | Uncategorized → Migrations |
Triage Stage: | Unreviewed → Accepted |
Type: | Uncategorized → Bug |
follow-up: 3 comment:2 by , 10 hours ago
Just to make sure before we commit to a solution.
Does Posgres still allows for generated field alterations but disallow that USING
is not used? If that's the case it seems like a better solution would be to have generated field alteration continue to be supported simply not specify USING
(which is effectively redundant with the column type) instead of disabling the feature?
-
django/db/backends/postgresql/schema.py
diff --git a/django/db/backends/postgresql/schema.py b/django/db/backends/postgresql/schema.py index 75bf331472..964009988c 100644
a b def _create_like_index_sql(self, model, field): 120 120 return None 121 121 122 122 def _using_sql(self, new_field, old_field): 123 if new_field.generated: 124 return "" 123 125 using_sql = " USING %(column)s::%(type)s" 124 126 new_internal_type = new_field.get_internal_type() 125 127 old_internal_type = old_field.get_internal_type(
comment:3 by , 9 hours ago
Replying to Simon Charette:
Just to make sure before we commit to a solution.
Does Posgres still allows for generated field alterations but disallow that
USING
is not used? If that's the case it seems like a better solution would be to have generated field alteration continue to be supported simply not specifyUSING
(which is effectively redundant with the column type) instead of disabling the feature?
I found that I can rename the GeneratedField without any issues.
Renaming a field within the expression throws the ValueError:
ValueError: Modifying GeneratedFields is not supported - the field sales.Order.total_cost must be removed and re-added with the new definition.
This error appears to be raised by Django, so it's not even getting to Postgres.
comment:4 by , 9 hours ago
It seems to be latter as the above patch works the adjusted tests provided by Sarah
-
tests/migrations/test_operations.py
diff --git a/tests/migrations/test_operations.py b/tests/migrations/test_operations.py index 3ac813b899..7aa3bfbc5d 100644
a b def _test_add_generated_field(self, db_persist): 6135 6135 operation.database_backwards(app_label, editor, new_state, project_state) 6136 6136 self.assertColumnNotExists(f"{app_label}_pony", "modified_pink") 6137 6137 6138 @skipUnlessDBFeature("supports_stored_generated_columns") 6139 def test_generated_field_changes_output_field(self): 6140 app_label = "test_gfcof" 6141 operation = migrations.AddField( 6142 "Pony", 6143 "modified_pink", 6144 models.GeneratedField( 6145 expression=F("pink") + F("pink"), 6146 output_field=models.IntegerField(), 6147 db_persist=True, 6148 ), 6149 ) 6150 from_state, to_state = self.make_test_state(app_label, operation) 6151 # Add generated column. 6152 with connection.schema_editor() as editor: 6153 operation.database_forwards(app_label, editor, from_state, to_state) 6154 # Update output_field used in the generated field. 6155 operation = migrations.AlterField( 6156 "Pony", 6157 "modified_pink", 6158 models.GeneratedField( 6159 expression=F("pink") + F("pink"), 6160 output_field=models.DecimalField(decimal_places=2, max_digits=16), 6161 db_persist=True, 6162 ), 6163 ) 6164 from_state = to_state.clone() 6165 to_state = self.apply_operations(app_label, from_state, [operation]) 6166 with connection.schema_editor() as editor: 6167 operation.database_forwards(app_label, editor, from_state, to_state) 6168 6138 6169 @skipUnlessDBFeature("supports_stored_generated_columns") 6139 6170 def test_add_generated_field_stored(self): 6140 6171 self._test_add_generated_field(db_persist=True)
comment:5 by , 9 hours ago
Cc: | added |
---|
comment:6 by , 9 hours ago
In summary what Postgres 16.5 changed is that you longer can specify USING
on ALTER COLUMN
for generated columns
ALTER TABLE "test_gfcof_pony" ALTER COLUMN "modified_pink" TYPE numeric(16, 2) USING "modified_pink"::numeric(16, 2);
Which makes sense as USING
is redundant with TYPE
and could result in further ambiguity more if you change the generated expression at the same time.
Well the solution appears to simply not specifying USING
when dealing with GeneratedField
alterations as suggested in comment:2
ALTER TABLE "test_gfcof_pony" ALTER COLUMN "modified_pink" TYPE numeric(16, 2);
Thank you for the detailed report
Replicated the error
tests/migrations/test_operations.py
Having this error instead makes sense to me.
CC-ed a couple of other folks who might have thoughts