DecimalRangeField __contains query for a value causes DataError

[Trent Holliday]

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/python@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.

[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.

[amansharma612]

PR - ​https://github.com/django/django/pull/18969.

It would be helpful if someone can indicate how to create tests for this change.

PS: Updated with tests

[Jacob Walls]

Thanks amansharma612, when you add test coverage you can unset "Needs Tests".

[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.

[Natalia Bidart]

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?

[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

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.