Postgres DecimalRangeField ignores bounds

[Jack Delany]

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.

[Mariusz Felisiak]

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.

[Jack Delany]

Replying to 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.

[Mariusz Felisiak]

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.