Opened 6 years ago

Closed 6 years ago

Last modified 6 years 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 6 years ago.
traceback
schema.diff​ (636 bytes ) - added by Tilman Koschnick 6 years ago.
patch

Download all attachments as: .zip

Change History (9)

by Tilman Koschnick, 6 years ago

Attachment: exception​ added

traceback

by Tilman Koschnick, 6 years ago

Attachment: schema.diff​ added

patch

comment:1 by Simon Charette, 6 years ago

Owner: changed from nobody to Simon Charette
Severity: Normal → Release blocker
Status: new → assigned
Triage Stage: Unreviewed → Accepted
Version: master → 2.2

comment:2 by Simon Charette, 6 years ago

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 by Simon Charette, 6 years ago

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 by Carlton Gibson, 6 years ago

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 by Mariusz Felisiak <felisiak.mariusz@…>, 6 years ago

Resolution: → fixed
Status: assigned → closed

In 386d89a:

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

Thanks Tilman Koschnick for the report and patch.

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

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 by Tilman Koschnick, 6 years ago

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

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