Opened 18 hours ago
Last modified 15 hours ago
#36173 assigned Bug
Usage of Concat with an explicit `output_field` results in unstable migrations changes
Reported by: | erchenstein | Owned by: | Simon Charette |
---|---|---|---|
Component: | Migrations | Version: | 5.1 |
Severity: | Normal | Keywords: | migrations concat |
Cc: | Triage Stage: | Accepted | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
I have a check constraint for a model that gets constantly dropped and added during migrations, even though there were no changes to the check constraint.
The bug occurs with Postgres and SQLite using Django 5.0 and Django 5.1.
The CheckConstraint checks whether the checksum of an identifier is correct via a modulo operation. The identifier is saved in a CharField. If the identifier starts with a number greater than 0, the formula is:
checksum = 97 – (int(identifier) % 97)
If the identifier starts with 0, a 2 has to be added to the identifier:
97 – (int("2" + identifier) % 97)
I use the ORM's Concat function in the check constraint to add the 2 at the beginning of the identifier in the latter case.
to concatenate strings. Therefore, I updated to Django 5.1, since | is used in >= 5.1. |
- I also tested in a test database, from which I deleted all identifiers starting with "0", to check whether the error would persist with a check constraint that didn' use Concat. It did not occur without using Concat.
- Because the original constraint was constructed by a function to reduce verbose code, I also tested whether a verbose/hardcoded version would lead to the error. It did.
- I used a test model that has only the identifier field and the CheckConstraint to see whether the error would occur. It did.
Checkconstraint:
identifier_checksum_check = (Q( IntegerFieldExact( ExpressionWrapper( Value(97) - Mod( Cast( Substr(F('identifier'), 1, 9), models.BigIntegerField() ), Value(97), output_field=models.BigIntegerField() ), output_field=models.BigIntegerField() ), Cast( Substr(F('identifier'), 10, 2), models.BigIntegerField() ) ) ) | Q( IntegerFieldExact( ExpressionWrapper( Value(97) - Mod( Cast( Concat(2, Substr(F('identifier'), 1, 9), output_field=models.CharField()), models.BigIntegerField() ), Value(97), output_field=models.BigIntegerField() ), output_field=models.BigIntegerField() ), Cast( Substr(F('identifier'), 10, 2), models.BigIntegerField() ) ) )) class TestModel(models.Model): identifier = models.CharField(max_length=11) class Meta: constraints = [ models.CheckConstraint( check=identifier_checksum_check, name="identifier_control_checksum_check", violation_error_message=_("The checksum is not correct!") ), ]
SQL:
Postgres check constraint generated by Django 5.0 using CONCAT
CHECK ((97 - mod("substring"(niss::text, 1, 9)::bigint, 97::bigint)) = "substring"(niss::text, 10, 2)::bigint OR (97 - mod(concat(2::text, "substring"(niss::text, 1, 9))::bigint, 97::bigint)) = "substring"(niss::text, 10, 2)::bigint)
CHECK ((97 - mod("substring"(niss::text, 1, 9)::bigint, 97::bigint)) = "substring"(niss::text, 10, 2)::bigint OR (97 - mod((COALESCE(2::text, ''::text) || COALESCE("substring"(niss::text, 1, 9), ''::text))::bigint, 97::bigint)) = "substring"(niss::text, 10, 2)::bigint)
Change History (2)
comment:1 by , 15 hours ago
Keywords: | migrations concat added; Migrations CheckConstraint CONCAT removed |
---|---|
Owner: | set to |
Status: | new → assigned |
Summary: | POSTGRES and SQLite migrations constantly DROP and ADD check constraint without changes to code - possibly due to use of CONCAT function (Django 5.x) → Usage of Concat with an explicit `output_field` results in unstable migrations changes |
Triage Stage: | Unreviewed → Accepted |
comment:2 by , 15 hours ago
Has patch: | set |
---|
This is is due to the implementation of
Expression.identity
not accounting for__init__
overrides that have*args
and**kwargs
captures. When this is the case the respective values (e.g.extra = {"output_field": CharField()}
in the case ofConcat
) must be recursively unpacked to ensuretype(output_field)
is used.From my understanding this has been a flaw of expression identity since their inception and was missed in #30628.
You can circumvent this problem by not passing an explicit
output_field
to yourConcat(2, Substr(F('identifier'), 1, 9))
.