Skip to content

Conversation

NoahStapp
Copy link
Contributor

No description provided.

@NoahStapp NoahStapp requested a review from a team as a code owner August 20, 2025 18:33
"""Converts Python :class:`decimal.Decimal` to BSON :class:`Decimal128`.

.. warning:: When converting BSON data types to and from built-in data types,
the possibility of data loss is always present due to mismatches in underlying implementations.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"possibility of data loss is always present" is extremely alarming. We have tests that do auto-convert and they don't have data loss issues so I think the language is too strong. Can we identify and explain exactly where the risk is and when it would occur?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Decimal128 supports up to 34 decimal digits of precision, decimal.Decimal by default supports up to 28 digit during arithmetic operations. When creating a new decimal.Decimal value, that 28 digit limit does not exist, so I don't think we need any warning here.

Copy link
Member

@ShaneHarvey ShaneHarvey Aug 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah it seems like the truncation issue will be caught on the encoding side:

>>> Decimal128(decimal.Decimal('1'*34))
Decimal128('1111111111111111111111111111111111')
>>> Decimal128(decimal.Decimal('1'*35))
Traceback (most recent call last):
  File "<python-input-6>", line 1, in <module>
    Decimal128(decimal.Decimal('1'*35))
    ~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/shane/git/mongo-python-driver/bson/decimal128.py", line 218, in __init__
    self.__high, self.__low = _decimal_to_128(value)
                              ~~~~~~~~~~~~~~~^^^^^^^
  File "/Users/shane/git/mongo-python-driver/bson/decimal128.py", line 76, in _decimal_to_128
    value = ctx.create_decimal(value)
decimal.Inexact: [<class 'decimal.Inexact'>]

Changes in Version 4.15.0 (XXXX/XX/XX)
--------------------------------------
.. warning:: When converting BSON data types to and from built-in data types, the possibility of data loss is always present
due to mismatches in underlying implementations.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure we need this warning here too.

return Decimal128(value)


class DecimalDecoder(TypeDecoder):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we add a docstring example showing out to use these classes?

sleepyStick
sleepyStick previously approved these changes Aug 20, 2025
Copy link
Contributor

@sleepyStick sleepyStick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@NoahStapp NoahStapp merged commit 5e96353 into mongodb:master Aug 21, 2025
79 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants