Re: [Django] #35575: Add support for constraint validation on GeneratedFields

[Django]

#35575: Add support for constraint validation on GeneratedFields
-------------------------------------+-------------------------------------
Reporter: Mark Gensler | Owner: Mark
| Gensler
Type: New feature | Status: assigned
Component: Database layer | Version: 5.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: generatedfield | Triage Stage: Accepted
uniqueconstraint checkconstraint |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mark Gensler):

Replying to [comment:4 Sarah Boyce]:
>
> My gut says option 1

Yes, that seems the simpler approach which directly addresses this problem
and no more. Also from the definition of a `GeneratedField` in the docs:

> A field that is always computed based on other fields in the model. This
field is managed and updated by the database itself.

I'll work on option 1.
--
Ticket URL: <https://code.djangoproject.com/ticket/35575#comment:5>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#35575: Add support for constraint validation on GeneratedFields
-------------------------------------+-------------------------------------
Reporter: Mark Gensler | Owner: Mark
| Gensler
Type: New feature | Status: assigned
Component: Database layer | Version: 5.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: generatedfield | Triage Stage: Accepted
uniqueconstraint checkconstraint |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Simon Charette):

I feel like we should avoid the naive approach of calling
`refresh_from_db(fields=generated_fields)` as that will mutate the in-
memory instance and leave it in this form as well as perform an extra
query per constraint.

What should be done IMO is for the query performed in `UniqueConstraint`
to call `replace_expressions({F(gfield.name):
gfield.expression.replace_expressions(....) for gfield in
generated_field})` so that will turn things like


{{{#!python
class Contributor(models.Model):
first_name = models.TextField()
last_name = models.TextField()
full_name = models.GeneratedField(
Lower(Concat("first_name", models.Value(" "), "last_name"))
)

class Meta:
constraints = {
UniqueConstraint(names="unique_full_name", MD5("full_name"))
}
}}}

Then the query to full clean the unique constraint, assuming `first_name`
and `last_name` are available, would be

{{{#!sql
SELECT 1 FROM contributor
WHERE md5(lower("first_name" || ' ' || "last_name")) = md5(lower('Mark' ||
' ' || 'Gensler')
}}}

The more we push to the database the less likely we are to run into race
conditions and serde roudtrip issues.
--
Ticket URL: <https://code.djangoproject.com/ticket/35575#comment:6>

[Django]

#35575: Add support for constraint validation on GeneratedFields
-------------------------------------+-------------------------------------
Reporter: Mark Gensler | Owner: Mark
| Gensler
Type: New feature | Status: assigned
Component: Database layer | Version: 5.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: generatedfield | Triage Stage: Accepted
uniqueconstraint checkconstraint |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Sarah Boyce):

I also think it's worth highlighting that this has come from us testing a
PR and not "organically". I don't think this is a quick win and we should
probably confirm that this is wanted, otherwise it might be best to
document that it isn't supported.
--
Ticket URL: <https://code.djangoproject.com/ticket/35575#comment:7>

[Django]

#35575: Add support for constraint validation on GeneratedFields
-------------------------------------+-------------------------------------
Reporter: Mark Gensler | Owner: Mark
| Gensler
Type: New feature | Status: assigned
Component: Database layer | Version: 5.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: generatedfield | Triage Stage: Accepted
uniqueconstraint checkconstraint |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Mark Gensler):

Replying to [comment:7 Sarah Boyce]:

> I also think it's worth highlighting that this has come from us testing
a PR and not "organically". I don't think this is a quick win and we
should probably confirm that this is wanted, otherwise it might be best to
document that it isn't supported.

How would we do that? Shall I raise a discussion on the django forum?

I don't see any gap in pure functionality at present. Any expression which
is defined for a `GeneratedField` could also be written directly as an
input to the `CheckConstraint` / `UniqueConstraint` /
`ExclusionConstraint`, rather than by using the `GeneratedField` as
shorthand for that expression. This change would simply allow a more
concise and readable definition of the constraint and assist with DRY. Or
is there another advantage which I'm missing?

If this isn't required, the only change necessary would be to prevent a
`GeneratedField` appearing in the expression of any constraints. Otherwise
a stale value would be returned for the `GeneratedField` attribute of an
instance during `constraint.validate()`. This wouldn't be strictly
backwards compatible, but users could re-write any existing constraints
which use a `GeneratedField`.

Replying to [comment:6 Simon Charette]:

Thanks for the examples and for raising `ExclusionConstraint`, I had
missed that. I'll investigate a solution using the outline you provided.
--
Ticket URL: <https://code.djangoproject.com/ticket/35575#comment:8>

[Django]

#35575: Add support for constraint validation on GeneratedFields
-------------------------------------+-------------------------------------
Reporter: Mark Gensler | Owner: Mark
| Gensler
Type: New feature | Status: assigned
Component: Database layer | Version: 5.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: generatedfield | Triage Stage: Accepted
uniqueconstraint checkconstraint |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Simon Charette):

FWIW I gave a shot at what I described above to ensure that it was
implementable and not too invasive and
[https://github.com/django/django/compare/main...charettes:django:ticket-35575
it was relatively straightforward]. I don't see why we shouldn't support
constraints over `GeneratedField` knowing that.
--
Ticket URL: <https://code.djangoproject.com/ticket/35575#comment:9>

[Django]

#35575: Add support for constraint validation on GeneratedFields
-------------------------------------+-------------------------------------
Reporter: Mark Gensler | Owner: Mark
| Gensler
Type: New feature | Status: assigned
Component: Database layer | Version: 5.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: generatedfield | Triage Stage: Accepted
uniqueconstraint checkconstraint |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Mark Gensler):

Replying to [comment:9 Simon Charette]:

> FWIW I gave a shot at what I described above to ensure that it was
implementable and not too invasive and
[https://github.com/django/django/compare/main...charettes:django:ticket-35575
it was relatively straightforward]. I don't see why we shouldn't support
constraints over `GeneratedField` knowing that.

Thanks Simon, I will use what you've done as a starting point for the PR.
--
Ticket URL: <https://code.djangoproject.com/ticket/35575#comment:10>