DecimalField.get_db_prep_value() doesn't call adapt_decimalfield_value()

[Tim Graham]

Some built-in fields have corresponding DatabaseOperations.adapt_<field_name>_value() methods. For all of these fields except DecimalField, get_db_prep_value() calls the corresponding "adapt" method.

DecimalField, however, was changed in e9814029f570bd0866dc859147bca90340bcc913 to call adapt_decimalfield_value() in get_db_prep_save(). This causes decimal values not to be converted when needed for query lookups (see ​hacky workaround in django-mongodb).

The issue also manifested when adapting ArrayField for MongoDB. The lack of a proper DecimalField.get_db_prep_value() means ​Decimals aren't prepared properly.

[Tim Graham]

​PR

[Simon Charette]

The report makes sense but just like any regression its warrants a test demonstrating it.

[Tim Graham]

I considered whether an additional test would be useful, but I'm not sure one could be created for the built-in database backends without resorting to mocking that checks that get_db_prep_value() calls adapt_decimalfield_value(). There are already many failures on django-mongodb without this patch (and removing the hacky workaround I mentioned in the description). Example:

======================================================================
ERROR: test_gt (lookup.test_decimalfield.DecimalFieldLookupTests.test_gt)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/tim/code/django/tests/lookup/test_decimalfield.py", line 25, in test_gt
    self.assertCountEqual(qs, [self.p3])
  File "/opt/python3.12.0/lib/python3.12/unittest/case.py", line 1198, in assertCountEqual
    first_seq, second_seq = list(first), list(second)
                            ^^^^^^^^^^^
  File "/home/tim/code/django/django/db/models/query.py", line 400, in __iter__
    self._fetch_all()
  File "/home/tim/code/django/django/db/models/query.py", line 1928, in _fetch_all
    self._result_cache = list(self._iterable_class(self))
                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/tim/code/django/django/db/models/query.py", line 91, in __iter__
    results = compiler.execute_sql(
              ^^^^^^^^^^^^^^^^^^^^^
  File "/home/tim/code/django-mongodb/django_mongodb/compiler.py", line 255, in execute_sql
    cursor = query.get_cursor()
             ^^^^^^^^^^^^^^^^^^
  File "/home/tim/code/django-mongodb/django_mongodb/query.py", line 19, in wrapper
    return func(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^
  File "/home/tim/code/django-mongodb/django_mongodb/query.py", line 74, in get_cursor
    return self.compiler.collection.aggregate(self.get_pipeline())
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/tim/.virtualenvs/django312/lib/python3.12/site-packages/pymongo/synchronous/collection.py", line 2958, in aggregate
    return self._aggregate(
           ^^^^^^^^^^^^^^^^
  File "/home/tim/.virtualenvs/django312/lib/python3.12/site-packages/pymongo/_csot.py", line 119, in csot_wrapper
    return func(self, *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/tim/.virtualenvs/django312/lib/python3.12/site-packages/pymongo/synchronous/collection.py", line 2866, in _aggregate
    return self._database.client._retryable_read(
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/tim/.virtualenvs/django312/lib/python3.12/site-packages/pymongo/synchronous/mongo_client.py", line 1863, in _retryable_read
    return self._retry_internal(
           ^^^^^^^^^^^^^^^^^^^^^
  File "/home/tim/.virtualenvs/django312/lib/python3.12/site-packages/pymongo/_csot.py", line 119, in csot_wrapper
    return func(self, *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/tim/.virtualenvs/django312/lib/python3.12/site-packages/pymongo/synchronous/mongo_client.py", line 1830, in _retry_internal
    ).run()
      ^^^^^
  File "/home/tim/.virtualenvs/django312/lib/python3.12/site-packages/pymongo/synchronous/mongo_client.py", line 2554, in run
    return self._read() if self._is_read else self._write()
           ^^^^^^^^^^^^
  File "/home/tim/.virtualenvs/django312/lib/python3.12/site-packages/pymongo/synchronous/mongo_client.py", line 2697, in _read
    return self._func(self._session, self._server, conn, read_pref)  # type: ignore
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/tim/.virtualenvs/django312/lib/python3.12/site-packages/pymongo/synchronous/aggregation.py", line 164, in get_cursor
    result = conn.command(
             ^^^^^^^^^^^^^
  File "/home/tim/.virtualenvs/django312/lib/python3.12/site-packages/pymongo/synchronous/helpers.py", line 45, in inner
    return func(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^
  File "/home/tim/.virtualenvs/django312/lib/python3.12/site-packages/pymongo/synchronous/pool.py", line 566, in command
    self._raise_connection_failure(error)
  File "/home/tim/.virtualenvs/django312/lib/python3.12/site-packages/pymongo/synchronous/pool.py", line 538, in command
    return command(
           ^^^^^^^^
  File "/home/tim/.virtualenvs/django312/lib/python3.12/site-packages/pymongo/synchronous/network.py", line 158, in command
    request_id, msg, size, max_doc_size = message._op_msg(
                                          ^^^^^^^^^^^^^^^^
  File "/home/tim/.virtualenvs/django312/lib/python3.12/site-packages/pymongo/message.py", line 415, in _op_msg
    return _op_msg_uncompressed(flags, command, identifier, docs, opts)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
bson.errors.InvalidDocument: cannot encode object: Decimal('0'), of type: <class 'decimal.Decimal'>

It raises the question of whether built-in database backends get special testing treatment. If this bug manifested on Oracle, for example, I don't think we would require a regression test for SQLite. I'm comfortable with running the existing tests with django-mongodb to ensure this issue does not regress.

[Simon Charette]

Ok so basically this was never an issue because the core backends PEP249 drivers (sqlite3, psycopg, mysqlclient, and oracledb) all support decimal.Decimal natively? If this is the case then it means the utils.format_number call is likely unnecessary at this point (​I'll give the suite a run by removing it) otherwise it means a regression test can be written for querying as well without relying on mocking.

[Sarah Boyce <42296566+sarahboyce@…>]

In b0b3024:

Refs #35982 -- Made BaseDatabaseOperations.adapt_decimalfield_value() a no-op.

[Sarah Boyce <42296566+sarahboyce@…>]

In 1860a1a:

Fixed #35982 -- Made DecimalField.get_db_prep_value() call DatabaseOperations.adapt_decimalfield_value().

Regression in e9814029f570bd0866dc859147bca90340bcc913.

Thanks Simon Charette for advice and review.