#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)
Change History (9)
Changed 5 years ago by
comment:1 Changed 5 years ago by
| Owner: | changed from nobody to Simon Charette |
|---|---|
| Severity: | Normal → Release blocker |
| Status: | new → assigned |
| Triage Stage: | Unreviewed → Accepted |
| Version: | master → 2.2 |
comment:2 Changed 5 years ago by
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 5 years ago by
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 5 years ago by
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)? 👍
traceback