[Django] #31872: Postgres DecimalRangeField ignores bounds
Django
--------------------------------------------+------------------------
Reporter: Jack Delany | Owner: (none)
Type: Uncategorized | Status: new
Component: contrib.postgres | Version: 3.1
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
--------------------------------------------+------------------------
Found in 2.2, still in master.
I believe there is an issue in the internal conversion/prep routine for
Range types.
{{{
from django.contrib.postgres.fields.ranges import DecimalRangeField
class OneModel(models.Model):
val = DecimalRangeField()
}}}
Given a simple object created from a tuple:
{{{
x = OneModel(val=(1.0, 2.0, "[]"))
x.save()
}}}
This emits the following SQL with an incorrect bounds:
{{{
INSERT INTO "app_onemodel" ("val") VALUES ('[1.0,2.0)') ...
}}}
Alternately:
{{{
from psycopg2.extras import NumericRange
x = OneModel(val=NumericRange(1.0, 2.0, "[]"))
x.save()
}}}
This one does what I expect and emits the following SQL and preserves the
bounds:
{{{
INSERT INTO "app_onemodel" ("val") VALUES ('[1.0,2.0]') ...
}}}
I tracked it down to:
{{{
django/contrib/postgres/fields/ranges.py:74:
def get_prep_value(self, value):
if value is None:
return None
elif isinstance(value, Range):
return value
elif isinstance(value, (list, tuple)):
# Problem is here
>>>> return self.range_type(value[0], value[1])
# Better: return self.range_type(*value)
return value
}}}
Given that get_prep_value() is calling through to the underlying psycopg2
object, I'd presume the intention is that the behavior matches the
underlying psycopg2 semantics.
Same pattern is found at line 86 in the ```RangeField.to_python()```
method.
I realize that by using (*value) it'll change the behavior if someone has
been passing tuples/list with more than three items, but the behavior
seems wrong in that case to silently ignore the extra items. One could be
more explicit and handle the two-item and three-item tuple/list cases
individually. That would minimize the changes.
There is another case #27147 that is related. In my experience, Ranges
generally work correctly and they do have the underlying Postgres behavior
(eg bounds normalizing for discreet ranges) so I'm not completely clear
what the issue is that #27147 is trying to address.
--
Ticket URL: <https://code.djangoproject.com/ticket/31872>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Jack Delany | Owner: (none)
Component: contrib.postgres | Version: 3.1
Severity: Normal | Resolution: wontfix
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by felixxm):
* status: new => closed
* type: Uncategorized => New feature
* resolution: => wontfix
Comment:
> x = OneModel(val=(1.0, 2.0, "[]"))
Thanks for this ticket, however this syntax has never been documented,
tested or supported. Moreover I don't think we should support it because
it's not readable. If you want to specify bounds you can use range types
from `psycopg2.extras`.
> I'd presume the intention is that the behavior matches the underlying
psycopg2 semantics.
We added it for simplicity not to match the psycopg2 semantic.
--
Ticket URL: <https://code.djangoproject.com/ticket/31872#comment:1>
Django
Reporter: Jack Delany | Owner: (none)
Component: contrib.postgres | Version: 3.1
Severity: Normal | Resolution: wontfix
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Jack Delany):
Replying to [comment:1 felixxm]:
> > x = OneModel(val=(1.0, 2.0, "[]"))
>
> Thanks for this ticket, however this syntax has never been documented,
tested or supported. Moreover I don't think we should support it because
it's not readable. If you want to specify bounds you can use range types
from `psycopg2.extras`.
>
> > I'd presume the intention is that the behavior matches the underlying
psycopg2 semantics.
>
> We added it for simplicity not to match the psycopg2 semantic.
Well two additional points.
First, is that the syntax is exactly the syntax that
psycopg2.extras.NumericRange supports, so it's not
undocumented/unsupported:
{{{
>>> from psycopg2.extras import NumericRange
>>> x=NumericRange(1.0, 2.0, '(]')
>>> x
NumericRange(1.0, 2.0, '(]')
>>> help(x)
Help on NumericRange in module psycopg2._range object:
class NumericRange(Range)
| NumericRange(lower=None, upper=None, bounds='[)', empty=False)
}}}
Second is that NumericRange objects are immutable. You can't modify them
after construction so the only way to create them is with that syntax.
So it seems to me that if you're going to allow that list/tuple input at
all it should do it correctly. You can't fix them after the fact.
The only thing that works correctly right now is to actually instantiate
and pass in a psycopg2 object as the Django value.
--
Ticket URL: <https://code.djangoproject.com/ticket/31872#comment:2>
Django
Reporter: Jack Delany | Owner: (none)
Component: contrib.postgres | Version: 3.1
Severity: Normal | Resolution: wontfix
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by felixxm):
> First, is that the syntax is exactly the syntax that
psycopg2.extras.NumericRange supports, so it's not
undocumented/unsupported.
I don't see in Django docs or tests any example of using 3-tuples with
bounds.
> The only thing that works correctly right now is to actually instantiate
and pass in a psycopg2 object as the Django value.
This ticket is about adding support for 3-tuples with bounds. If you can
describe any behavior that is documented, tested and supported but doesn't
work properly, please do.
--
Ticket URL: <https://code.djangoproject.com/ticket/31872#comment:3>