Extra validation introduced in 30581 breaks certain constraint setups

[Ionel Cristian Mărieș]

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.

[Mariusz Felisiak]

.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.

[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.

[Ionel Cristian Mărieș]

I consider this a regression in Django.

[Ionel Cristian Mărieș]

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 :-)

[Simon Charette]

I think I understand what's going on here.

The reporter 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.

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 ​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. Model.validate_constraints chooses to validate all of them as it doesn't build on top of this ordering assumption.

To me this is a case of wontfix to avoid having users continue to build flawed assumptions about the order of definitions of their Meta.constraints entries.

[Mariusz Felisiak]

I agree with Simon. As for as I'm aware, aborting on the first ValidationError is not an option.