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>

[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: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Mark Gensler):

* has_patch: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/35575#comment:11>

[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: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Mark Gensler):

I have added a draft PR but there are still a couple of outstanding
issues. I thought the PR would probably be a better place for
review/discussion.

Simon, would you be able to take a look? Thanks Mark
--
Ticket URL: <https://code.djangoproject.com/ticket/35575#comment:12>

[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: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Simon Charette):

The patch looks ready to be reviewed to me. I requested that a release
note be added for 5.0.X just in case we wanted to backport but in the
light of #35560 disabling validation at first I think we should just
consider this a new feature for inclusion in 5.2.x. That would also gives
us enough time to properly address #34871 by then without restricting
ourselves to solutions that are not to invasive for a backport. Given any
of the `GeneratedField` validation can be emulated with expression
constraints as Mark pointed out it should allow for users to lean into
that in the mean time.

IMO the branch demonstrates that enabling this feature is achievable
without too much boilerplate an ensures that `GeneratedField` works just
like any other `Field` with regards to `Meta.constraints` instead of
crashing with `IntegrityError`.
--
Ticket URL: <https://code.djangoproject.com/ticket/35575#comment:13>

[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: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Mariusz Felisiak):

> I think we should just consider this a new feature for inclusion in
5.2.x.

For me, definitely a new feature.
--
Ticket URL: <https://code.djangoproject.com/ticket/35575#comment:14>

[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: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Sarah Boyce):

* needs_better_patch: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/35575#comment:15>

[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: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Sarah Boyce):

* needs_better_patch: 1 => 0

--
Ticket URL: <https://code.djangoproject.com/ticket/35575#comment:16>

[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: Ready for
uniqueconstraint checkconstraint | checkin

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Sarah Boyce):

* stage: Accepted => Ready for checkin

--
Ticket URL: <https://code.djangoproject.com/ticket/35575#comment:17>

[Django]

#35575: Add support for constraint validation on GeneratedFields
-------------------------------------+-------------------------------------
Reporter: Mark Gensler | Owner: Mark
| Gensler

Type: New feature | Status: closed

Component: Database layer | Version: 5.0
(models, ORM) |

Severity: Normal | Resolution: fixed

Keywords: generatedfield | Triage Stage: Ready for
uniqueconstraint checkconstraint | checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Sarah Boyce <42296566+sarahboyce@…>):

* resolution: => fixed
* status: assigned => closed

Comment:

In [changeset:"228128618bd895ecad235d2215f4ad4e3232595d" 2281286]:
{{{#!CommitTicketReference repository=""
revision="228128618bd895ecad235d2215f4ad4e3232595d"
Fixed #35575 -- Added support for constraint validation on
GeneratedFields.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/35575#comment:18>