Opened 9 months ago

Closed 9 months ago

Last modified 9 months ago

#30258 closed Bug (fixed)

Failed to add CheckConstraint on IntegerRangeField

Reported by: Tilman Koschnick Owned by: Simon Charette
Component: Database layer (models, ORM) Version: 2.2
Severity: Release blocker Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Adding a CheckConstraint like:

constraints = (
    m.CheckConstraint(check=m.Q(precipitation_probability__contained_by=NumericRange(0, 101)), name='precipitation_percent_range'),
)

resulted in the following error (full traceback attached):

django.db.utils.ProgrammingError: syntax error at or near "object"
LINE 1: ...obability" <@ <psycopg2._range.NumberRangeAdapter object at ...

The attached patch fixes the issue for me.

Attachments (2)

exception (3.5 KB) - added by Tilman Koschnick 9 months ago.
traceback
schema.diff (636 bytes) - added by Tilman Koschnick 9 months ago.
patch

Download all attachments as: .zip

Change History (9)

Changed 9 months ago by Tilman Koschnick

Attachment: exception added

traceback

Changed 9 months ago by Tilman Koschnick

Attachment: schema.diff added

patch

comment:1 Changed 9 months ago by Simon Charette

Owner: changed from nobody to Simon Charette
Severity: NormalRelease blocker
Status: newassigned
Triage Stage: UnreviewedAccepted
Version: master2.2

comment:2 Changed 9 months ago by Simon Charette

Thanks for your report and patch, I turned it into a PR.

Happy to give you attribution if you can provide an email alias.

On a different subject this patch unveiled what I believe is a design flaw with Index and Constraint's create_sql and remove_sql methods. They should return a tuple of SQL and a parms instead of a baked SQL string so we can let the backend deal with the escaping itself. I'll spin up a different PR that does just that.

comment:3 Changed 9 months ago by Simon Charette

I gave the create_sql/constraint_sql methods returning (sql, params) a go but it would require a non-trivial refactor of deferred_sql. I still think it would be worthy to make and might even be required in the future to handle the quoting on backends that don't expose low level quoting facilities like psycopg2 does. The number of reports regarding similar crashes now that CheckConstraint, UniqueConstraint(condition), and Index(condition) are out there will be good indicator of whether or not this is required.

comment:4 Changed 9 months ago by Carlton Gibson

Hey Simon, thanks for pulling this into the PR!

I still think it would be worthy to make ...

Fancy creating a separate ticket (at your leisure)? 👍

comment:5 Changed 9 months ago by Mariusz Felisiak <felisiak.mariusz@…>

Resolution: fixed
Status: assignedclosed

In 386d89a:

Fixed #30258 -- Adjusted postgres schema value quoting of ranges.

Thanks Tilman Koschnick for the report and patch.

comment:6 Changed 9 months ago by Mariusz Felisiak <felisiak.mariusz@…>

In 883d870:

[2.2.x] Fixed #30258 -- Adjusted postgres schema value quoting of ranges.

Thanks Tilman Koschnick for the report and patch.

Backport of 386d89ab55e620440d30590a8a104fe6d5eef830 from master

comment:7 Changed 9 months ago by Tilman Koschnick

Hi, thanks for looking into this, and getting the fix into rc1!

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