[Django] #35676: ExclusionConstraint that includes a ForeignKey fails validation in ModelForm

[Django]

#35676: ExclusionConstraint that includes a ForeignKey fails validation in
ModelForm
-----------------------------+-----------------------------------------
Reporter: diesieben07 | Type: Uncategorized
Status: new | Component: Uncategorized
Version: 5.1 | Severity: Normal
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-----------------------------+-----------------------------------------
I'm not sure if the summary above is good, but let me demonstrate:

{{{
class BlockedDaysConfig(models.Model):
name = models.CharField(max_length=255)


class BlockedDaysRange(models.Model):
config = models.ForeignKey(BlockedDaysConfig,
on_delete=models.CASCADE, related_name='days')
date_range = DateRangeField()

class Meta:
constraints = (
ExclusionConstraint(
name='%(app_label)s_%(class)s_no_overlapping_ranges_per_config',
expressions=(
('config_id', RangeOperators.EQUAL, ),
('date_range', RangeOperators.OVERLAPS, ),
)
),
)
}}}

`BlockedDaysRange` represents a range of blocked days. `BlockedDaysConfig`
represents a range of these blocked days. Example:
A BlockedDaysConfig with name "School Holidays in Germany" contains
several BlockedDaysConfig entries, one for "summer holidays", one for
"winter holidays", etc.
The exclusion constraint validates that the date ranges within one config
do not overlap.

Now I have configured a ModelAdmin for this:

{{{
class BlockedDaysRangeInline(admin.TabularInline):
model = BlockedDaysRange


@admin.register(BlockedDaysConfig)
class BlockedDaysConfigAdmin(admin.ModelAdmin):
inlines = (BlockedDaysRangeInline, )
}}}

The problem now happens when saving, because `BaseModelForm._post_clean`
will ''exclude'' the `InlineForeignKey` for `config` when calling
`full_clean` (see
https://github.com/django/django/blob/b99c608ea10cabc97a6b251cdb6e81ef2a83bdcf/django/forms/models.py#L477-L488).
Because `config` is in `exclude`, when `ExclusionConstraint` eventually
calls `_get_field_value_map`, the `config` is not included and thus a
nonsensical SQL query happens (`WHERE config_id=config_id and date_range
&& '[2024-01-01,2024-01-03)'::daterange`). This will now validate against
''any'' date ranges, not just ones with the same `config_id`.
As you can see from the comment in the code snippet linked above, this
problem is already recognized for unique constraints, which are handled
specially by `ModelForm`.
--
Ticket URL: <https://code.djangoproject.com/ticket/35676>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#35676: ExclusionConstraint that includes a ForeignKey fails validation in
ModelForm

------------------------------+------------------------------------
Reporter: Take Weiland | Owner: (none)
Type: Bug | Status: new
Component: Forms | Version: 5.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0

------------------------------+------------------------------------
Changes (by Simon Charette):

* component: Uncategorized => Forms
* stage: Unreviewed => Accepted
* type: Uncategorized => Bug

Comment:

Thanks for the detailed report.
[https://github.com/django/django/blob/b99c608ea10cabc97a6b251cdb6e81ef2a83bdcf/django/forms/models.py#L477-L488
The comment you pointed at] makes it clear that this is a variant of
#12960 but for constraints instead of uniques.

I think something along the following lines should address the issue but
I'm curious to hear your thoughts on it.

{{{#!diff
diff --git a/django/forms/models.py b/django/forms/models.py
index 8084e16c8d..bbb4d94e33 100644
--- a/django/forms/models.py
+++ b/django/forms/models.py
@@ -370,10 +370,12 @@ def __init__(
# if initial was provided, it should override the values from
instance
if initial is not None:
object_data.update(initial)
- # self._validate_unique will be set to True by
BaseModelForm.clean().
- # It is False by default so overriding self.clean() and failing
to call
- # super will stop validate_unique from being called.
+ # self._validate_(unique|constraints) will be set to True by
+ # BaseModelForm.clean(). It is False by default so overriding
+ # self.clean() and failing to call super will stop
+ # validate_(unique|constraints) from being called.
self._validate_unique = False
+ self._validate_constraints = False
super().__init__(
data,
files,
@@ -436,6 +438,7 @@ def _get_validation_exclusions(self):

def clean(self):
self._validate_unique = True
+ self._validate_constraints = True
return self.cleaned_data

def _update_errors(self, errors):
@@ -495,13 +498,17 @@ def _post_clean(self):
self._update_errors(e)

try:
- self.instance.full_clean(exclude=exclude,
validate_unique=False)
+ self.instance.full_clean(
+ exclude=exclude, validate_unique=False,
validate_constraints=False
+ )
except ValidationError as e:
self._update_errors(e)

- # Validate uniqueness if needed.
+ # Validate uniqueness and constraints if needed.
if self._validate_unique:
self.validate_unique()
+ if self._validate_constraints:
+ self.validate_constraints()

def validate_unique(self):
"""
@@ -514,6 +521,17 @@ def validate_unique(self):
except ValidationError as e:
self._update_errors(e)

+ def validate_constraints(self):
+ """
+ Call the instance's validate_constraints() method and update the
form's
+ validation errors if any were raised.
+ """
+ exclude = self._get_validation_exclusions()
+ try:
+ self.instance.validate_constraints(exclude=exclude)
+ except ValidationError as e:
+ self._update_errors(e)
+
def _save_m2m(self):
"""
Save the many-to-many fields and generic relations for this form.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/35676#comment:1>

[Django]

#35676: ExclusionConstraint that includes a ForeignKey fails validation in
ModelForm
------------------------------+------------------------------------
Reporter: Take Weiland | Owner: (none)
Type: Bug | Status: new
Component: Forms | Version: 5.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
------------------------------+------------------------------------

Comment (by Take Weiland):

Yes, that definitely does seem sensible.
The only potential issue might be that when creating new objects then the
foreign key field will still be None, because the related object isn't
saved to the DB yet.
In my crude hack to work around this (override
`Model.validate_constraints` and remove the field from `exclude`) this
works fine, however I don't know if there are situations where it doesn't.
In my case (shown above) it will result in a `qs.filter(config=None)` to
happen. As I understand it, this doesn't do anything, but lookups other
than `exact` might not like a `None` there.
--
Ticket URL: <https://code.djangoproject.com/ticket/35676#comment:2>

[Django]

#35676: ExclusionConstraint that includes a ForeignKey fails validation in
ModelForm
------------------------------+------------------------------------
Reporter: Take Weiland | Owner: (none)
Type: Bug | Status: new
Component: Forms | Version: 5.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
------------------------------+------------------------------------

Changes (by Simon Charette):

* cc: Simon Charette (added)

Comment:

> The only potential issue might be that when creating new objects then
the foreign key field will still be None, because the related object isn't
saved to the DB yet.

That's another can of worm and an issue that is unfortunately pretty hard
to solve. The way this is emulated in today for unique constraints
validation in admin inlines can be found in `ModelFormSet.validate_unique`
[https://github.com/django/django/blob/b99c608ea10cabc97a6b251cdb6e81ef2a83bdcf/django/forms/models.py#L803-L860
source].

The gist of it is that
[https://github.com/django/django/blob/b99c608ea10cabc97a6b251cdb6e81ef2a83bdcf/django/forms/models.py#L803-L860
for each fields marked to be unique within the formset] all associated
values are combined and
[https://github.com/django/django/blob/b99c608ea10cabc97a6b251cdb6e81ef2a83bdcf/django/forms/models.py#L830
the name of the field is used to make duplicate values collide]. This is
poorly implemented IMO as it doesn't account for database level uniqueness
(e.g. if a field has a specific `db_collation` that is case insensitive
then `"foo" == "Foo"` and only a transit through the database can
determine that)

If you wanted to manually implement a similar emulation for exclusion
constraints you could define a `BaseInlineFormSet` subclass and override
its `clean` method to iterate through all forms and ensure that each form
contains non-overlapping `date_range` in Python. This subclass could then
be assigned to `BlockedDaysRangeInline.formset`.

What I'm curious to hear about is whether the patch above prevents the
problematic SQL query from being executed in the first place.
--
Ticket URL: <https://code.djangoproject.com/ticket/35676#comment:3>

[Django]

#35676: ExclusionConstraint that includes a ForeignKey fails validation in
ModelForm
------------------------------+------------------------------------
Reporter: Take Weiland | Owner: (none)
Type: Bug | Status: new
Component: Forms | Version: 5.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
------------------------------+------------------------------------

Comment (by Take Weiland):

I've manually applied your patch to a custom ModelForm subclass and it
does solve the problematic SQL query.
When altering existing objects, the query happens correctly (with the
correct config ID substituted). When creating a new object, it will do a
`WHERE config_id = NULL AND date_range && (...)`. That doesn't cause any
problems though, because the = NULL comparison will just always yield NULL
and thus no rows will be returned.
--
Ticket URL: <https://code.djangoproject.com/ticket/35676#comment:4>

[Django]

#35676: ExclusionConstraint that includes a ForeignKey fails validation in
ModelForm

------------------------------+-------------------------------------
Reporter: Take Weiland | Owner: Calvin Vu
Type: Bug | Status: assigned

Component: Forms | Version: 5.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0

------------------------------+-------------------------------------
Changes (by Calvin Vu):

* owner: (none) => Calvin Vu
* status: new => assigned

--
Ticket URL: <https://code.djangoproject.com/ticket/35676#comment:5>

[Django]

#35676: ExclusionConstraint that includes a ForeignKey fails validation in
ModelForm

------------------------------+------------------------------------
Reporter: Take Weiland | Owner: (none)
Type: Bug | Status: new

Component: Forms | Version: 5.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
------------------------------+------------------------------------

Changes (by Calvin Vu):

* owner: Calvin Vu => (none)
* status: assigned => new

--
Ticket URL: <https://code.djangoproject.com/ticket/35676#comment:6>

[Django]

#35676: ExclusionConstraint that includes a ForeignKey fails validation in
ModelForm

------------------------------+-----------------------------------------
Reporter: Take Weiland | Owner: Lufafa Joshua
Type: Bug | Status: assigned

Component: Forms | Version: 5.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0

------------------------------+-----------------------------------------
Changes (by Lufafa Joshua):

* owner: (none) => Lufafa Joshua

* status: new => assigned

--
Ticket URL: <https://code.djangoproject.com/ticket/35676#comment:7>

[Django]

#35676: ExclusionConstraint that includes a ForeignKey fails validation in
ModelForm

------------------------------+------------------------------------
Reporter: Take Weiland | Owner: (none)
Type: Bug | Status: new

Component: Forms | Version: 5.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
------------------------------+------------------------------------

Changes (by Lufafa Joshua):

* owner: Lufafa Joshua => (none)

* status: assigned => new

--
Ticket URL: <https://code.djangoproject.com/ticket/35676#comment:8>

[Django]

#35676: ExclusionConstraint that includes a ForeignKey fails validation in
ModelForm
------------------------------+------------------------------------
Reporter: Take Weiland | Owner: (none)
Type: Bug | Status: new
Component: Forms | Version: 5.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
------------------------------+------------------------------------
Changes (by Lufafa Joshua):

* cc: Lufafa Joshua (added)

--
Ticket URL: <https://code.djangoproject.com/ticket/35676#comment:9>

[Django]

#35676: ExclusionConstraint that includes a ForeignKey fails validation in
ModelForm

------------------------------+-------------------------------------
Reporter: Take Weiland | Owner: Calvin Vu
Type: Bug | Status: assigned

Component: Forms | Version: 5.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0

------------------------------+-------------------------------------
Changes (by Calvin Vu):

* cc: Calvin Vu (added)

* owner: (none) => Calvin Vu

* status: new => assigned

--
Ticket URL: <https://code.djangoproject.com/ticket/35676#comment:10>

[Django]

#35676: ExclusionConstraint that includes a ForeignKey fails validation in
ModelForm

------------------------------+------------------------------------
Reporter: Take Weiland | Owner: (none)
Type: Bug | Status: new

Component: Forms | Version: 5.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
------------------------------+------------------------------------

Changes (by Calvin Vu):

* cc: Calvin Vu (removed)
* owner: Calvin Vu => (none)

* status: assigned => new

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

[Django]

#35676: ExclusionConstraint that includes a ForeignKey fails validation in
ModelForm
------------------------------+------------------------------------
Reporter: Take Weiland | Owner: (none)
Type: Bug | Status: new
Component: Forms | Version: 5.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
------------------------------+------------------------------------

Changes (by Clifford Gama):

* cc: Clifford Gama (added)

Comment:

Marked #36128 as partial duplicate.

Can I open a PR with the suggested patch?
--
Ticket URL: <https://code.djangoproject.com/ticket/35676#comment:12>

[Django]

#35676: ExclusionConstraint that includes a ForeignKey fails validation in
ModelForm
------------------------------+------------------------------------
Reporter: Take Weiland | Owner: (none)
Type: Bug | Status: new
Component: Forms | Version: 5.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
------------------------------+------------------------------------

Comment (by Simon Charette):

Sure thing Clifford, feel free to assign the ticket to yours! Thanks for
looking into it.
--
Ticket URL: <https://code.djangoproject.com/ticket/35676#comment:13>

[Django]

#35676: ExclusionConstraint that includes a ForeignKey fails validation in
ModelForm

------------------------------+-----------------------------------------
Reporter: Take Weiland | Owner: Clifford Gama
Type: Bug | Status: assigned

Component: Forms | Version: 5.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0

------------------------------+-----------------------------------------
Changes (by Clifford Gama):

* owner: (none) => Clifford Gama

* status: new => assigned

Comment:

Thanks!
--
Ticket URL: <https://code.djangoproject.com/ticket/35676#comment:14>

[Django]

#35676: ExclusionConstraint that includes a ForeignKey fails validation in
ModelForm
------------------------------+-----------------------------------------
Reporter: Take Weiland | Owner: Clifford Gama
Type: Bug | Status: assigned
Component: Forms | Version: 5.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
------------------------------+-----------------------------------------
Changes (by Clifford Gama):

* has_patch: 0 => 1

Comment:

https://github.com/django/django/pull/19157
--
Ticket URL: <https://code.djangoproject.com/ticket/35676#comment:15>

[Django]

#35676: ExclusionConstraint that includes a ForeignKey fails validation in
ModelForm
------------------------------+-----------------------------------------
Reporter: Take Weiland | Owner: Clifford Gama
Type: Bug | Status: assigned
Component: Forms | Version: 5.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
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/35676#comment:16>

[Django]

#35676: ExclusionConstraint that includes a ForeignKey fails validation in
ModelForm
------------------------------+-----------------------------------------
Reporter: Take Weiland | Owner: Clifford Gama
Type: Bug | Status: assigned
Component: Forms | Version: 5.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0

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

Changes (by Clifford Gama):

* needs_better_patch: 1 => 0

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

[Django]

#35676: ExclusionConstraint that includes a ForeignKey fails validation in
ModelForm
------------------------------+-----------------------------------------
Reporter: Take Weiland | Owner: Clifford Gama
Type: Bug | Status: assigned
Component: Forms | Version: 5.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
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/35676#comment:18>

[Django]

#35676: ExclusionConstraint that includes a ForeignKey fails validation in
ModelForm
------------------------------+-----------------------------------------
Reporter: Take Weiland | Owner: Clifford Gama
Type: Bug | Status: assigned
Component: Forms | Version: 5.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0

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

Changes (by Clifford Gama):

* needs_better_patch: 1 => 0

--
Ticket URL: <https://code.djangoproject.com/ticket/35676#comment:19>

[Django]

#35676: ExclusionConstraint that includes a ForeignKey fails validation in
ModelForm

-------------------------------------+-------------------------------------

Reporter: Take Weiland | Owner: Clifford
| Gama
Type: Bug | Status: assigned
Component: Forms | Version: 5.1
Severity: Normal | Resolution:

Keywords: | Triage Stage: Ready for
| 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/35676#comment:20>

[Django]

#35676: ExclusionConstraint that includes a ForeignKey fails validation in
ModelForm
-------------------------------------+-------------------------------------
Reporter: Take Weiland | Owner: Clifford
| Gama

Type: Bug | Status: closed
Component: Forms | Version: 5.1
Severity: Normal | Resolution: fixed

Keywords: | Triage Stage: Ready for
| 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:"0ebea6e5c07485a36862e9b6e2be18d1694ad2c5" 0ebea6e5]:
{{{#!CommitTicketReference repository=""
revision="0ebea6e5c07485a36862e9b6e2be18d1694ad2c5"
Fixed #35676 -- Made BaseModelForm validate constraints that reference an
InlineForeignKeyField.

Co-authored-by: Simon Charette <chare...@gmail.com>
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/35676#comment:21>

[Django]

#35676: ExclusionConstraint that includes a ForeignKey fails validation in
ModelForm
-------------------------------------+-------------------------------------
Reporter: Take Weiland | Owner: Clifford
| Gama
Type: Bug | Status: closed
Component: Forms | Version: 5.1
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by GitHub <noreply@…>):

In [changeset:"3306b7283bc76a4e0cd647776fdc8b02d38d0935" 3306b728]:
{{{#!CommitTicketReference repository=""
revision="3306b7283bc76a4e0cd647776fdc8b02d38d0935"
Refs #35676 -- Added supports_table_check_constraints skip to model_forms
tests.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/35676#comment:22>