#34293 closed Bug (wontfix)

Extra validation introduced in 30581 breaks certain constraint setups

Reported by: Ionel Cristian Mărieș Owned by: nobody
Component: Database layer (models, ORM) Version: 4.1
Severity: Normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

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.

Change History (6)

comment:1 by Mariusz Felisiak, 22 months ago

Component: UncategorizedDatabase layer (models, ORM)
Resolution: invalid
Status: newclosed
Type: UncategorizedBug

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

comment:2 by Ionel Cristian Mărieș, 22 months ago

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.

comment:3 by Ionel Cristian Mărieș, 22 months ago

Resolution: invalid
Status: closednew

I consider this a regression in Django.

comment:4 by Ionel Cristian Mărieș, 22 months ago

Version: 4.24.1

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

comment:5 by Simon Charette, 22 months ago

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.

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.

Version 0, edited 22 months ago by Simon Charette (next)

comment:6 by Mariusz Felisiak, 22 months ago

Resolution: wontfix
Status: newclosed

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

Note: See TracTickets for help on using tickets.
Back to Top