[Django] #34293: Extra validation introduced in 30581 breaks certain constraint setups
Django
-------------------------------------------------+------------------------
Reporter: Ionel Cristian Mărieș | Owner: nobody
Type: Uncategorized | Status: new
Component: Uncategorized | Version: 4.2
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 |
-------------------------------------------------+------------------------
So I have this model
{{{
class TimeTsTzRange(models.Func):
function = 'timetstzrange'
output_field = fields.DateTimeRangeField()
class Schedule(models.Model):
class Meta:
constraints = [
models.CheckConstraint(
name='schedule_start_lt_end',
check=(
Q(start_time__lt=F('end_time')) |
Q(start_time__lte=HALF_HOUR_BEFORE_MIDNIGHT,
end_time=MIDNIGHT)
),
),
constraints.ExclusionConstraint(
name='schedule_exclude_overlapping',
expressions=[
(
TimeTsTzRange(
'start_time',
'end_time',
fields.RangeBoundary(inclusive_upper=True),
),
fields.RangeOperators.OVERLAPS,
),
('weekday', fields.RangeOperators.EQUAL),
('therapist', fields.RangeOperators.EQUAL),
],
),
]
start_time = models.TimeField(
verbose_name=_('start time'),
)
end_time = models.TimeField(
verbose_name=_('end time'),
)
# and some other irrelevant fields
}}}
Under 3.2 this worked fine,
Schedule.objects.create(start_time=time(2),end_time=time(1)) would raise
an IntegrityError as expected with a decently helpful message: new row for
relation "app_schedule" violates check constraint "schedule_start_lt_end"
DETAIL: Failing row contains (13, 3, 02:00:00, 01:00:00, 3).
Under 4.2a1 this raises django.db.utils.DataError with a not really
helpful message: range lower bound must be less than or equal to range
upper bound
CONTEXT: PL/pgSQL function timetstzrange(time without time zone,time
without time zone,text) line 6 at RETURN
Now this happens because in 4.2 all the constraints are checked,
regardless if 1st one faile, and obviously the second will fail horribly.
I've tried adding deferrable=models.Deferrable.DEFERRED to the second
contraint but it's still run in validation - maybe this could be changed?
And then my problem would go away.
Sure, I could make a custom function (that wraps timetstzrange) to
shortcircuit the second constraint if range is invalid but that seems ugly
if not dangerous.
--
Ticket URL: <https://code.djangoproject.com/ticket/34293>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Ionel Cristian | Owner: nobody
Mărieș |
Type: Bug | Status: closed
Component: Database layer | Version: 4.2
(models, ORM) |
Severity: Normal | Resolution: invalid
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* status: new => closed
* resolution: => invalid
* component: Uncategorized => Database layer (models, ORM)
* type: Uncategorized => Bug
Comment:
`.create()` doesn't automatically call constraints validation, so it
behaves exactly the same in both Django 3.2 and 4.2a1 and crashes with
`IntegrityError`.
--
Ticket URL: <https://code.djangoproject.com/ticket/34293#comment:1>
Django
-------------------------------------+-------------------------------------
Reporter: Ionel Cristian | Owner: nobody
Mărieș |
Type: Bug | Status: closed
Component: Database layer | Version: 4.2
(models, ORM) |
Severity: Normal | Resolution: invalid
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Ionel Cristian Mărieș):
Whoops, I misreported what triggers this. The error is the same but it's
not create, it's full_clean:
{{{
Schedule(start_time=time(2), end_time=time(1)).full_clean()
}}}
In 3.2 that raised a ValidationError
In 4.2 it raises that DataError
I'd expect this would create nasty 500 error responses in form processing.
Correct me if I'm wrong.
Still need a way to deal with this, otherwise I can't upgrade to 4.2.
--
Ticket URL: <https://code.djangoproject.com/ticket/34293#comment:2>
Django
-------------------------------------+-------------------------------------
Reporter: Ionel Cristian | Owner: nobody
Mărieș |
Component: Database layer | Version: 4.2
(models, ORM) |
Severity: Normal | Resolution:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: closed => new
* resolution: invalid =>
Comment:
I consider this a regression in Django.
--
Ticket URL: <https://code.djangoproject.com/ticket/34293#comment:3>
Django
-------------------------------------+-------------------------------------
Reporter: Ionel Cristian | Owner: nobody
Mărieș |
Type: Bug | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Ionel Cristian Mărieș):
* version: 4.2 => 4.1
Comment:
I'm so sorry, I tested some more and it turns out it reproduces on 4.1 too
(4.0 is fine).
Not sure what to say now except that I'd like a non-ugly solution :-)
--
Ticket URL: <https://code.djangoproject.com/ticket/34293#comment:4>
Django
-------------------------------------+-------------------------------------
Reporter: Ionel Cristian | Owner: nobody
Mărieș |
Type: Bug | Status: new
Component: Database layer | Version: 4.1
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Simon Charette):
I think I understand what's going on here.
The reported defined constraints in a way makes assumption that
`schedule_start_lt_end` will always be enforced before
`schedule_exclude_overlapping` is which depends on the order of creation
of constraints on the table and the order of the members of
`Meta.constraints`.
I think this might be an argument to make `Model.validate_constraint`
abort on the first `ValidationError` it encounters. The alternative
solution would be document that ''dependent'' constraints should use
`conditions` so they can be skipped on invalid input.
For example, in this case `schedule_exclude_overlapping` should be defined
in the following manner since it uses the `timetstzrange` function which
expects that ''range lower bound must be less than or equal to range upper
bound'' as pointed out in the (un)helpful message.
{{{#!python
constraints.ExclusionConstraint(
name='schedule_exclude_overlapping',
expressions=[
(
TimeTsTzRange(
'start_time',
'end_time',
fields.RangeBoundary(inclusive_upper=True),
),
fields.RangeOperators.OVERLAPS,
),
('weekday', fields.RangeOperators.EQUAL),
('therapist', fields.RangeOperators.EQUAL),
],
condition=(
Q(start_time__lt=F('end_time')) |
Q(start_time__lte=HALF_HOUR_BEFORE_MIDNIGHT, end_time=MIDNIGHT)
)
)
}}}
I would argue that this is more of a user error since
[https://www.postgresql.org/docs/15/ddl-constraints.html PostgreSQL
doesn't offer guarantees about the order in which it will run constraints]
> The order doesn't matter. It does not necessarily determine in which
order the constraints are checked.
which means that depending on the order in which PostgreSQL choose to
check constraints the user could either get an `IntegrityError` or a
`DataError` depending which attempting to insert data that violates one
constraint and provides invalid data for the other.
--
Ticket URL: <https://code.djangoproject.com/ticket/34293#comment:5>
Django
-------------------------------------+-------------------------------------
Reporter: Ionel Cristian | Owner: nobody
Mărieș |
Component: Database layer | Version: 4.1
(models, ORM) |
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => closed
* resolution: => wontfix
Comment:
I agree with Simon. As for as I'm aware, aborting on the first
`ValidationError` is not an option.
--
Ticket URL: <https://code.djangoproject.com/ticket/34293#comment:6>