[Django] #36031: DecimalRangeField __contains query for a value causes DataError

[Django]

#36031: DecimalRangeField __contains query for a value causes DataError
-----------------------------+--------------------------------------------
Reporter: trumpet2012 | Type: Bug
Status: new | Component: contrib.postgres
Version: 4.2 | 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
-----------------------------+--------------------------------------------
With a model of:


{{{
class Thing(models.Model):
decimal_range = DecimalRangeField()
}}}

Attempting to query against the decimal range field using a value instead
of a range of values leads to the below error being raised.

{{{
>>> list(Thing.objects.filter(decimal_range__contains=199))
Traceback (most recent call last):
File "/Users/tholliday/code/ADMS/backend/.venv/lib/python3.10/site-
packages/django/db/backends/utils.py", line 89, in _execute
return self.cursor.execute(sql, params)
psycopg2.errors.InvalidTextRepresentation: invalid input syntax for type
integer: "none"
LINE 1: ...HERE "contractor_thing"."decimal_range" @> (199)::numeric(No...
^


The above exception was the direct cause of the following exception:

Traceback (most recent call last):
File
"/opt/homebrew/Cellar/pyt...@3.10/3.10.14_1/Frameworks/Python.framework/Versions/3.10/lib/python3.10/code.py",
line 90, in runcode
exec(code, self.locals)
File "<console>", line 1, in <module>
File "/Users/tholliday/code/ADMS/backend/.venv/lib/python3.10/site-
packages/django/db/models/query.py", line 398, in __iter__
self._fetch_all()
File "/Users/tholliday/code/ADMS/backend/.venv/lib/python3.10/site-
packages/django/db/models/query.py", line 1881, in _fetch_all
self._result_cache = list(self._iterable_class(self))
File "/Users/tholliday/code/ADMS/backend/.venv/lib/python3.10/site-
packages/django/db/models/query.py", line 91, in __iter__
results = compiler.execute_sql(
File "/Users/tholliday/code/ADMS/backend/.venv/lib/python3.10/site-
packages/django/db/models/sql/compiler.py", line 1562, in execute_sql
cursor.execute(sql, params)
File "/Users/tholliday/code/ADMS/backend/.venv/lib/python3.10/site-
packages/django/db/backends/utils.py", line 102, in execute
return super().execute(sql, params)
File "/Users/tholliday/code/ADMS/backend/.venv/lib/python3.10/site-
packages/cachalot/monkey_patch.py", line 137, in inner
return original(cursor, sql, *args, **kwargs)
File "/Users/tholliday/code/ADMS/backend/.venv/lib/python3.10/site-
packages/sentry_sdk/integrations/django/__init__.py", line 644, in execute
result = real_execute(self, sql, params)
File "/Users/tholliday/code/ADMS/backend/.venv/lib/python3.10/site-
packages/django/db/backends/utils.py", line 67, in execute
return self._execute_with_wrappers(
File "/Users/tholliday/code/ADMS/backend/.venv/lib/python3.10/site-
packages/django/db/backends/utils.py", line 80, in _execute_with_wrappers
return executor(sql, params, many, context)
File "/Users/tholliday/code/ADMS/backend/.venv/lib/python3.10/site-
packages/django/db/backends/utils.py", line 84, in _execute
with self.db.wrap_database_errors:
File "/Users/tholliday/code/ADMS/backend/.venv/lib/python3.10/site-
packages/django/db/utils.py", line 91, in __exit__
raise dj_exc_value.with_traceback(traceback) from exc_value
File "/Users/tholliday/code/ADMS/backend/.venv/lib/python3.10/site-
packages/django/db/backends/utils.py", line 89, in _execute
return self.cursor.execute(sql, params)
django.db.utils.DataError: invalid input syntax for type integer: "none"
LINE 1: ...HERE "contractor_thing"."decimal_range" @> (199)::numeric(No...
}}}

I've done some digging to try and identify why this is happening. From
what I can tell, when the `DecimalRangeField` gets intialized,
`self.base_field` is a `DecimalField` and gets instantiated with
`decimal_places` and `max_digits` set to None,
https://github.com/django/django/blame/fcd9d08379a2aee3b2c49eab0d0b8db6fd66d091/django/contrib/postgres/fields/ranges.py#L65.
Then when the `Cast` operation happens for the `RangeContains` lookup
here:
https://github.com/django/django/blame/fcd9d08379a2aee3b2c49eab0d0b8db6fd66d091/django/contrib/postgres/fields/ranges.py#L215
it results in passing in the None values for the precision and scale.
--
Ticket URL: <https://code.djangoproject.com/ticket/36031>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#36031: DecimalRangeField __contains query for a value causes DataError

-------------------------------------+-------------------------------------
Reporter: Trent Holliday | Owner: Chaitanya
| Rahalkar
Type: Bug | Status: assigned
Component: contrib.postgres | Version: 4.2
Severity: Normal | Resolution:

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 Chaitanya Rahalkar):

* owner: (none) => Chaitanya Rahalkar
* status: new => assigned

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

[Django]

#36031: DecimalRangeField __contains query for a value causes DataError

----------------------------------+--------------------------------------
Reporter: Trent Holliday | Owner: (none)
Type: Bug | Status: new

Component: contrib.postgres | Version: 4.2
Severity: Normal | Resolution:
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 Chaitanya Rahalkar):

* owner: Chaitanya Rahalkar => (none)
* status: assigned => new

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

[Django]

#36031: DecimalRangeField __contains query for a value causes DataError
----------------------------------+--------------------------------------
Reporter: Trent Holliday | Owner: (none)
Type: Bug | Status: new
Component: contrib.postgres | Version: 4.2
Severity: Normal | Resolution:
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 amansharma612):

I think the mentioned cause is correct. Note that
[https://github.com/django/django/blob/fcd9d08379a2aee3b2c49eab0d0b8db6fd66d091/django/db/backends/postgresql/base.py#L104]
maps to the column type to "numeric(%(max_digits)s, %(decimal_places)s)",
and the fields are populated here
[https://github.com/django/django/blob/fcd9d08379a2aee3b2c49eab0d0b8db6fd66d091/django/db/models/fields/__init__.py#L886].

Though I am not sure of a way to fix this, but would be happy to open a PR
if given direction.
--
Ticket URL: <https://code.djangoproject.com/ticket/36031#comment:3>

[Django]

#36031: DecimalRangeField __contains query for a value causes DataError
----------------------------------+--------------------------------------
Reporter: Trent Holliday | Owner: (none)
Type: Bug | Status: new
Component: contrib.postgres | Version: 4.2
Severity: Normal | Resolution:
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 amansharma612):

PR - [https://github.com/django/django/pull/18969].

It would be helpful if someone can indicate how to create tests for this
change.
--
Ticket URL: <https://code.djangoproject.com/ticket/36031#comment:4>

[Django]

#36031: DecimalRangeField __contains query for a value causes DataError

-------------------------------------+-------------------------------------
Reporter: Trent Holliday | Owner:

| amansharma612
Type: Bug | Status: assigned

Component: contrib.postgres | Version: 4.2
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed

Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0

-------------------------------------+-------------------------------------
Changes (by amansharma612):

* has_patch: 0 => 1
* needs_tests: 0 => 1
* owner: (none) => amansharma612

* status: new => assigned

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

[Django]

#36031: DecimalRangeField __contains query for a value causes DataError
-------------------------------------+-------------------------------------
Reporter: Trent Holliday | Owner:
| amansharma612
Type: Bug | Status: assigned
Component: contrib.postgres | Version: 4.2
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Jacob Walls):

* needs_tests: 1 => 0

Comment:

Thanks amansharma612, when you add test coverage you can unset "Needs
Tests".
--
Ticket URL: <https://code.djangoproject.com/ticket/36031#comment:6>

[Django]

#36031: DecimalRangeField __contains query for a value causes DataError
-------------------------------------+-------------------------------------
Reporter: Trent Holliday | Owner:
| amansharma612
Type: Bug | Status: assigned
Component: contrib.postgres | Version: 4.2
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Trent Holliday):

Thanks for the patch amansharma612.

The proposed changes have the potential for issues when querying against
the range, if the base `DecimalField` for the range is instantiated with
`max_digits` and `decimal_places`. When attempting to query using the
`__contains` with a decimal that exceeds the fields precision you get the
following error:


{{{
NumericValueOutOfRange: numeric field overflow
DETAIL: A field with precision 5, scale 2 must round to an absolute value
less than 10^3.

File "django/db/backends/utils.py", line 89, in _execute
return self.cursor.execute(sql, params)

DataError: numeric field overflow
DETAIL: A field with precision 5, scale 2 must round to an absolute value
less than 10^3.
}}}


This is something we are doing in a custom `DecimalRangeField` so that we
can limit the values entered and get good form validation. We currently
have a workaround in place where we implement a custom lookup that casts
the value while ignoring the base fields precision.

Our current workaround which gets registered to our custom decimal range
field:

{{{
class _UnboundedDecimalField(models.DecimalField):

def db_type(self, connection):
return "numeric"


class UnboundedDecimalRangeContains(RangeContains):

def get_prep_lookup(self):
if not isinstance(self.rhs, (list, tuple, Range)):
return Cast(self.rhs, output_field=_UnboundedDecimalField())
return super().get_prep_lookup()
}}}

I recognize this is probably out of scope for Django since the Django
provided decimal range field would not have this problem. However it does
lead to unexpected issues when extending the provided field and
implementing that precision. One possible way to avoid this would be to
always have the decimal range field cast the value in the contains lookup
as the more generic `numeric` to avoid the precision error when querying
against the range.
--
Ticket URL: <https://code.djangoproject.com/ticket/36031#comment:7>

[Django]

#36031: DecimalRangeField __contains query for a value causes DataError
-------------------------------------+-------------------------------------
Reporter: Trent Holliday | Owner:
| amansharma612
Type: Bug | Status: assigned

Component: contrib.postgres | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Natalia Bidart):

* cc: Simon Charette, Mariusz Felisiak (added)
* stage: Unreviewed => Accepted
* version: 4.2 => dev

Comment:

I've been reading the comments and analyzing this ticket to complete the
triage. I think this is a valid tickets, I can reproduce the issue.

But to me, the correct solution is to be to ensure that a
`DecimalRangeField` is created with valid `max_digits` and
`decimal_places`. This would mean extending the `DecimalRangeField` to
receive and use these parameters, and adding system checks that mimic the
current ones for `DecimalField`:

{{{
$ python -Wall manage.py makemigrations
SystemCheckError: System check identified some issues:

ERRORS:
testapp.Ticket36031Model.decimal_plain: (fields.E130) DecimalFields must
define a 'decimal_places' attribute.
testapp.Ticket36031Model.decimal_plain: (fields.E132) DecimalFields must
define a 'max_digits' attribute.
}}}

Simon, Mariusz, what do you think?
--
Ticket URL: <https://code.djangoproject.com/ticket/36031#comment:8>

[Django]

#36031: DecimalRangeField __contains query for a value causes DataError
-------------------------------------+-------------------------------------
Reporter: Trent Holliday | Owner:
| amansharma612
Type: Bug | Status: assigned
Component: contrib.postgres | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Simon Charette):

Given the Postgres `numrange` type is continuous (it doesn't enforce a
discrete step) and thus doesn't allow defining `max_digits` or
`decimal_places` I would find it strange that we expose such attributes
that only happen to be used as a side effect of how the
`RangeContains.get_prep_lookup` method is implemented.

Adding these attributes would give a false sense that doing things like

{{{#!python
class Product(models.Model):
price_range = DecimalRangeField(max_digits=5, decimal_places=2)

Product.objects.create(price_range=["1999.99", "99999.99"])
}}}

Would be disallowed while `numrange` and the current implementation of
`DecimalRangeField` allows such value.

In other words, adding proper support for `max_digits` and
`decimal_places` would require quite a lot of work to get right as we
can't delegate to Postgres to get a discrete `numrange` which I don't
think is warranted here. We should either create a specialized
`DecimalRangeContains` lookup that casts to `::numeric` or go forward with
the proposed solution to have `Cast(expr, DecimalField())` to work (which
other users have also ran into #32593).

This lack of `numeric` parametrization is something we missed when we
introduced `DecimalRangeField` to replace the bogus `FloatRangeField`
(also backed by `numrange`) in #29598.
--
Ticket URL: <https://code.djangoproject.com/ticket/36031#comment:9>

[Django]

#36031: DecimalRangeField __contains query for a value causes DataError
-------------------------------------+-------------------------------------
Reporter: Trent Holliday | Owner:
| amansharma612
Type: Bug | Status: assigned
Component: contrib.postgres | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 1 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Sarah Boyce):

* needs_tests: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/36031#comment:10>