Fixed #35038 - Changing violation_error_message on constraints cause a remove/add operation in migration

[adriennefranke]

Fixes #35038. I created a method within the BaseConstraint class that returns a copy of the Constraint object without the two fields (violation_error_code and violation_error_message) that do not affect the DDL. I then call this method in create_altered_constraints and use that to determine when a migration for constraints is truly necessary.

There was another PR that was opened to fix this ticket so here's a link to that for reference.

[charettes]

Just like verbose_name changes on Field result in AlterField we'll want to track and autodetect these changes as well.

If is not already a noop at the schema application level we should make one but all attributes should be tracked.

In order words, the solution here should not be to avoid detecting changes to some attribute of Constraint but to make sure that making non-DDL changes to a Constraint doesn't result in the constraint being dropped and created again.

It seems like we should have an AlterConstraint operation that is in charge of dealing with that but it doesn't exist already 😬

This one is really a pickle, I can see the appeal to do away with discarding some attributes to avoid introducing AlterConstraint for the sake of supporting these options but it makes this code alien to the remaining of the migration code base; it's not only the auto-detector that cares about deconstruction and equality.

Maybe it's time to revisit the discussion around not support constraint alteration in the first place and making them

Refs ticket-25253

[adriennefranke]

@charettes Thanks for explaining. I don't want to introduce code that's different in its design from the rest of the migration code so I'm happy to work on AlterConstraint if you think that's the best path forward.

[charettes]

@adriennefranke I think it warrants further discussions that I didn't see mentioned on the ticket so I figured I'd chime in.

Whether or not adding AlterConstraint is worth the trouble to keep things coherent around the code base should at least be discussed with a larger audience as we've taken an explicit stance with regards to options that don't affect underyling DDL in the past and I don't think that this aspect was considered when Constraint, AddConstraint, RemoveConstraint were added as Constraint.validate was added later on.

[adriennefranke]

@charettes What would you say the right next steps are? Should I bring it up on the Discord or in the forums?

[charettes]

I think the forum would be a good location to start a discussion on that yes.

[50-Course]

Hi @adriennefranke, first timer here on django core - hopped here from the djangocon US talks.

Thanks for the patch, and I think its really an interesting one following the trend of conversations from #17644 and ticket-25253.

In addition, to @charettes, could I suggest sending in a discourse update on the regarding this patch to discord as well?

[adriennefranke]

@50-Course Yes, I will do that as well. Thanks!

[felixxm]

It seems like we should have an AlterConstraint operation that is in charge of dealing with that but it doesn't exist already 😬

Yes, yes, yes, we need AlterConstraint clearly documented as a no-op operation.

[adriennefranke]

Going to close this PR while I work on the AlterConstraint path. Thanks for the guidance everyone.