[Django] #33344: Define bounds transform for discrete postgres range fields

Django

unread,

Dec 7, 2021, 10:59:01 AM12/7/21

to django-...@googlegroups.com

#33344: Define bounds transform for discrete postgres range fields
-------------------------------------+-------------------------------------
Reporter: Vidir | Owner: (none)
Valberg Gudmundsson |
Type: New | Status: new
feature |
Component: | Version: dev
contrib.postgres |
Severity: Normal | Keywords:
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
The fix for https://code.djangoproject.com/ticket/27147 adressed the issue
of non-discrete range types. Left is the issue of how the ranges are
"represented" when brought back from the database.

At $WORK we have a model like so:

{{{
class Foo(models.Model):
...
period = models.DateRangeField()
}}}

The problem arises when we display the periods of multiple `Foo`s. Using
the default bound of `[)` returned by postgres means that adjacent `Foo`s
"visually" share a date in the upper of one and lower of the other. We
could of course handle this ''each time'' we output the value by
decrementing the upper value. But this is a bit error prone and can lead
to confusions.

I propose that we add the following to
`django.contrib.postres.fields.ranges` (heavily inspired by Jakub
Skałeckis comment at
https://code.djangoproject.com/ticket/27147#comment:8):

{{{
class DiscreteRangeField(RangeField):

def __init__(self, *args, bounds_transform=CANONICAL_RANGE_BOUNDS,
**kwargs):
if bounds_transform not in ALLOWED_BOUNDS:
raise ValueError("bounds_transform must be one of '[)', '(]',
'()', or '[]'.")
self.bounds_transform = bounds_transform
super().__init__(*args, **kwargs)

def from_db_value(self, value, expression, connection):
if value is None:
return

if self.bounds_transform[0] == "(" and value.lower:
value._lower = value.lower - self.bounds_transform_unit

if self.bounds_transform[1] == "]" and value.upper:
value._lower = value.upper - self.bounds_transform_unit

value._bounds = self.bounds_transform
return value
}}}

and make `IntegerRangeField`, `BigIntegerRangeField` and `DateRangeField`
inherit from `DiscreteRangeField`.

I have already written some tests, and if there are no big gotchas to this
approach I would love to submit a PR in the next couple of days.

--
Ticket URL: <https://code.djangoproject.com/ticket/33344>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,

Dec 7, 2021, 11:00:50 AM12/7/21

to django-...@googlegroups.com

#33344: Define bounds transform for discrete postgres range fields
-------------------------------------+-------------------------------------

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Description changed by Vidir Valberg Gudmundsson:

Old description:

New description:

The fix for https://code.djangoproject.com/ticket/27147 adressed the issue
of non-discrete range types. Left is the issue of how the ranges are
"represented" when brought back from the database.

At $WORK we have a model like so:

{{{
class Foo(models.Model):
...
period = models.DateRangeField()
}}}

The problem arises when we display the periods of multiple `Foo`s. Using
the default bound of `[)` returned by postgres means that adjacent `Foo`s
"visually" share a date in the upper of one and lower of the other. We
could of course handle this ''each time'' we output the value by
decrementing the upper value. But this is a bit error prone and can lead
to confusions.

I propose that we add something in the lines of the following to

{{{
class DiscreteRangeField(RangeField):

--

--
Ticket URL: <https://code.djangoproject.com/ticket/33344#comment:1>

Django

unread,

Dec 7, 2021, 11:01:03 AM12/7/21

to django-...@googlegroups.com

#33344: Define bounds transform for discrete postgres range fields
-------------------------------------+-------------------------------------

Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Description changed by Vidir Valberg Gudmundsson:

Old description:

> The fix for https://code.djangoproject.com/ticket/27147 adressed the

> issue of non-discrete range types. Left is the issue of how the ranges
> are "represented" when brought back from the database.
>
> At $WORK we have a model like so:
>
> {{{
> class Foo(models.Model):
> ...
> period = models.DateRangeField()
> }}}
>

>
> The problem arises when we display the periods of multiple `Foo`s. Using
> the default bound of `[)` returned by postgres means that adjacent `Foo`s
> "visually" share a date in the upper of one and lower of the other. We
> could of course handle this ''each time'' we output the value by
> decrementing the upper value. But this is a bit error prone and can lead
> to confusions.
>

> I propose that we add something in the lines of the following to

> `django.contrib.postres.fields.ranges` (heavily inspired by Jakub
> Skałeckis comment at
> https://code.djangoproject.com/ticket/27147#comment:8):
>
> {{{
> class DiscreteRangeField(RangeField):
>
> def __init__(self, *args, bounds_transform=CANONICAL_RANGE_BOUNDS,
> **kwargs):
> if bounds_transform not in ALLOWED_BOUNDS:
> raise ValueError("bounds_transform must be one of '[)', '(]',
> '()', or '[]'.")
> self.bounds_transform = bounds_transform
> super().__init__(*args, **kwargs)
>
> def from_db_value(self, value, expression, connection):
> if value is None:
> return
>
> if self.bounds_transform[0] == "(" and value.lower:
> value._lower = value.lower - self.bounds_transform_unit
>
> if self.bounds_transform[1] == "]" and value.upper:
> value._lower = value.upper - self.bounds_transform_unit
>
> value._bounds = self.bounds_transform
> return value
> }}}
>
> and make `IntegerRangeField`, `BigIntegerRangeField` and `DateRangeField`
> inherit from `DiscreteRangeField`.
>
> I have already written some tests, and if there are no big gotchas to
> this approach I would love to submit a PR in the next couple of days.

New description:

The fix for https://code.djangoproject.com/ticket/27147 adressed the issue
of non-discrete range types. Left is the issue of how the ranges are
"represented" when brought back from the database.

At $WORK we have a model like so:

{{{
class Foo(models.Model):
...
period = models.DateRangeField()
}}}

The problem arises when we display the periods of multiple `Foo`s. Using
the default bound of `[)` returned by postgres means that adjacent `Foo`s
"visually" share a date in the upper of one and lower of the other. We
could of course handle this ''each time'' we output the value by
decrementing the upper value. But this is a bit error prone and can lead
to confusions.

I propose that we add something along the lines of the following to

{{{
class DiscreteRangeField(RangeField):

--

--
Ticket URL: <https://code.djangoproject.com/ticket/33344#comment:2>

Django

unread,

Dec 7, 2021, 1:09:35 PM12/7/21

to django-...@googlegroups.com

#33344: Define bounds transform for discrete postgres range fields
-------------------------------------+-------------------------------------

Reporter: Vidir Valberg | Owner: (none)
Gudmundsson |

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Mariusz Felisiak):

* status: new => closed
* resolution: => needsinfo

Comment:

Thanks for this ticket, however it's not clear for me what do you want to
achieve. Bounds cannot be specify for discrete range types because they
are not natively supported on PostgreSQL. I don't think we should add
implicit logic in our fields to give the impression that they are, this
looks error-prone and it's probably not worth the complexity.

--
Ticket URL: <https://code.djangoproject.com/ticket/33344#comment:3>

Django

unread,

Dec 7, 2021, 2:11:04 PM12/7/21

to django-...@googlegroups.com

#33344: Define bounds transform for discrete postgres range fields
-------------------------------------+-------------------------------------

Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Mariusz Felisiak):

Moreover this can be confusing for users who will assume that values are
stored with different bounds.

--
Ticket URL: <https://code.djangoproject.com/ticket/33344#comment:4>

Django

unread,

Dec 7, 2021, 2:11:48 PM12/7/21

to django-...@googlegroups.com

#33344: Define bounds transform for discrete postgres range fields
-------------------------------------+-------------------------------------

Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Vidir Valberg Gudmundsson):

Replying to [comment:3 Mariusz Felisiak]:

> Thanks for this ticket, however it's not clear for me what do you want
to achieve. Bounds cannot be specify for discrete range types because they
are not natively supported on PostgreSQL. I don't think we should add
implicit logic in our fields to give the impression that they are, this
looks error-prone and it's probably not worth the complexity.

Fair enough - although I want to try to motivate it some more to try to
see if there is any way we can solve this problem:

1. I have a date range 2021-01-01 to 2021-01-31.
2. I want to save this in postgresql (in a `DateRangeField`), thus doing:
{{{
>>> foo.period = DateRange(lower=date(2021, 1, 1), upper=date(2021, 1,
31), bounds="[]")
}}}
3. We retrieving this from the database it comes back as
{{{
>>> foo.period
DateRange(lower=date(2021, 1, 1), upper=date(2021, 2, 1), bounds="[)")
}}}
4. So when I want to represent this in, say, a template, I have to
explicitly do the following every time I want to access the upper field.
{{{
foo.period.upper - timedelta(day=1)
}}}

So my goal is not to specify the bounds going into postgres, but only how
the data coming out is represented. Hence the naming of `bounds_transform`
(I started calling it `bounds_cast`).

Does this make my goal more clear?

--
Ticket URL: <https://code.djangoproject.com/ticket/33344#comment:5>

Django

unread,

Dec 7, 2021, 2:13:15 PM12/7/21

to django-...@googlegroups.com

#33344: Define bounds transform for discrete postgres range fields
-------------------------------------+-------------------------------------

Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Vidir Valberg Gudmundsson):

Replying to [comment:4 Mariusz Felisiak]:

> Moreover this can be confusing for users who will assume that values are
stored with different bounds.

Yeah, that I can agree on. That's also why I want to somehow name the
feature along the lines of "transform", "cast" og maybe "output". Also the
default would be "[)" thus only something one would set if one wanted to
change the output behaviour.

--
Ticket URL: <https://code.djangoproject.com/ticket/33344#comment:6>

Reply all

Reply to author

Forward