Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#33905 closed Bug (fixed)

Validation of check constraints against ranges involving PostgreSQL's lower()/upper() functions produces invalid SQL.

Reported by: David Sanders Owned by: David Sanders
Component: contrib.postgres Version: 4.1
Severity: Release blocker Keywords:
Cc: Gagaro Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by David Sanders)

Given a model using the Postgres field IntegerRangeField:

class Sample(models.Model):
    value_range = IntegerRangeField(null=True, blank=True)

    class Meta:
        constraints = [
            CheckConstraint(
                name="positive_value_range",
                check=Q(value_range__startswith__gte=0),
            ),
        ]

validation of the check constraint produces a DatabaseError:

sample = Sample()
sample.validate_constraints()

Got a database error calling check() on <Q: (AND: (AND: ('value_range__startswith__gte', 0)))>: operator does not exist: text >= integer
LINE 1: SELECT 1 AS "_check" WHERE lower(NULL) >= 0
                                               ^
HINT:  No operator matches the given name and argument types. You might need to add explicit type casts.

This is because Postgres' lower() and upper() functions are overloaded to accept either text or range types - the former for converting case with the latter retrieving the lower/upper bounds of the range.

When an expression LOWER(NULL) is encountered without an underlying column Postgres assumes the resulting expression is text:

postgres=# select UPPER(NULL);
 upper
-------
 NULL!
(1 row)

postgres=# \gdesc
 Column | Type
--------+------
 upper  | text
(1 row)

I doubt this is a serious error though because upon inspecting the new constraint validation code any DatabaseError is caught and a warning is logged but it would be nice to have a functioning check for this.

I have written a test & small patch which pretty much just adapts some existing operand casting code for Postgres (I haven't signed the CLA yet though because I'm not sure if someone else might want to dry it up a bit with a common mixin?)

Change History (8)

comment:2 by David Sanders, 2 years ago

Description: modified (diff)

comment:3 by David Sanders, 2 years ago

Summary: Validation of check constraints involving Postgres' lower()/upper() functions produce invalid sqlValidation of check constraints against ranges involving Postgres' lower()/upper() functions produce invalid sql

comment:4 by Mariusz Felisiak, 2 years ago

Cc: Gagaro added
Component: Uncategorizedcontrib.postgres
Needs documentation: set
Needs tests: set
Owner: changed from nobody to David Sanders
Patch needs improvement: set
Severity: NormalRelease blocker
Status: newassigned
Summary: Validation of check constraints against ranges involving Postgres' lower()/upper() functions produce invalid sqlValidation of check constraints against ranges involving PostgreSQL's lower()/upper() functions produces invalid SQL.
Triage Stage: UnreviewedAccepted
Type: UncategorizedBug
Version: dev4.1

Thanks for the report! Bug in 667105877e6723c6985399803a364848891513cc.

comment:5 by Mariusz Felisiak, 2 years ago

Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset
Triage Stage: AcceptedReady for checkin

comment:6 by Mariusz Felisiak <felisiak.mariusz@…>, 2 years ago

In e0ac72fe:

Refs #33905 -- Added test for CheckConstraint() validation with ArrayField and contains.

comment:7 by Mariusz Felisiak <felisiak.mariusz@…>, 2 years ago

Resolution: fixed
Status: assignedclosed

In e0ae1363:

Fixed #33905 -- Fixed CheckConstraint() validation on range fields.

Bug in 667105877e6723c6985399803a364848891513cc.

comment:8 by Mariusz Felisiak <felisiak.mariusz@…>, 2 years ago

In e215948f:

[4.1.x] Fixed #33905 -- Fixed CheckConstraint() validation on range fields.

Bug in 667105877e6723c6985399803a364848891513cc.

Backport of e0ae1363ec2aa71945be26f869cafd4181ccbc95 from main

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