Add Support for Decimal BSON Type (Decimal128Field)

gbroques commented 5 years ago

MongoDB added a new native type for handling floating point numbers called NumberDecimal in 3.4.

See the following documentation for details: https://docs.mongodb.com/manual/tutorial/model-monetary-data/#using-the-decimal-bson-type

MongoEngine lacks a corresponding field that allows you to store decimal values as a NumberDecimal.

{
    "total" : NumberDecimal("31.73")
}

The closest two fields are DecimalField and FloatField which store values as a decimal, and is unsuitable for monetary arithmetic (subject to rounding errors) .

{
    "total" : 31.73
}

There are some comments on other issues that relate to this one:

How would we implement this?

Anyone have ideas on the best way to add support for this?

I think adding a new NumberDecimalField field may be appropriate.

We could call the new field CurrencyField to emphasize this field is suitable for monetary arithmetic.

Also, since NumberDecimal is only supported in MongoDB 3.4 and greater, how do we handle it in the library? Do we just put a warning in the documentation, or raise an error if they try using the field with a version of MongoDB < 3.4?

What's the recommended interim solution for modeling currency values with MongoEngine?

The Scale Factor Approach

One approach is to use IntField and the Scale Factor approach.

What's a Work-Around for Storing Values as NumberDecimal with MongoEngine?

Ideally, I'd like a work-around to store values using NumberDecimal as the Scale Factor approach has overhead involved.

According to this comment, if you cast the values to bson.Decimal128 before inserting into MongoDB, then the values are stored as NumberDecimal.

I tried overloading the save method on my Model, but the value still appears to be stored as a decimal float value in MongoDB.

    def save(self, *args, **kwargs):
        self['total'] = Decimal128(self['total'])
        super().save(*args, **kwargs)

{
    "total" : 31.73
}

Workaround

Kind of hacky, but this seems to work in a pinch.

from bson.decimal128 import Decimal128

class Decimal128Field(StringField):
    """128-bit decimal-based floating-point field capable of emulating decimal rounding with exact precision.
    Stores the value as a `NumberDecimal` intended for monetary data, such as financial, tax, and scientific computations.
    """

    def to_mongo(self, value):
        if isinstance(value, Decimal128):
            return value
        return Decimal128(value)

    def to_python(self, value):
        return self.to_mongo(value)

    def validate(self, value):
        if not isinstance(value, Decimal128):
            try:
                value = Decimal128(value)
            except (TypeError, ValueError) as exc:
                self.error('Could not convert value to Decimal128: %s' % exc)

    def prepare_query_value(self, op, value):
        return super(Decimal128Field, self).prepare_query_value(op, self.to_mongo(value))

class SomeDocument(Document):
    total = Decimal128Field(required=True)

toniree commented 4 years ago

Just checking in to see if you guys support NumberDecimal yet

gbroques commented 4 years ago

@toniree I no longer actively use MongoEngine, and it seems like this issue never received any attention.

My work-around was documented at the bottom of the issue (see the Decimal128Field class).

I hope that helps :)

bagerard commented 3 years ago

Before I dig more on the topic, maybe you can save me some time and answer a few questions, do you know if bson.Decimal128 is compatible with python decimal.Decimal? Like if they can be used interchangeably ?

Because if decimal.Decimal is some sort of a superset of bson.Decimal128, we have to consider to keep the conversion to Decimal128 hidden from the users and do the conversion only when talking to the database. But this implies there is no precision loss or differences when going back and forth between decimal.Decimal and bson.Decimal128.

In fact, I'm expecting developers to use decimal.Decimal when setting the attributes, meaning they might expect Decimal back when objects are reloaded.

For future ref in this discussion - pymodm chose to return bson.Decimal128 (code). I havn't checked the other mongo ORM out there

jschlyter commented 3 years ago

I think @janste63 might know more about Decimal/Decimal128.

jschlyter commented 3 years ago

After thinking a bit about this, I believe it would be less confusing for developers to use decimal.Decimal as the input/output of the Decimal128Field, just as we do with other types. E.g., we don't have bson.Timestamp for DateTimeField – we have datetime.

bagerard commented 3 years ago

I'm also leaning towards that direction, for future ref marrow.mongo (another mongo ORM) has taken that direction (code)

The bson doc highlights some potential incompatibilities and probably we should make use of create_decimal128_context(), and to_decimal()

jschlyter commented 3 years ago

See also https://github.com/jschlyter/mongoengine/compare/decimal128field...jschlyter:decimal128field_decimal?expand=1 for a version of this PR using Decimal.

bagerard commented 3 years ago

Let's move forward with having that field returning Decimal and applying conversion to Decimal128 only when interacting with the database

jschlyter commented 3 years ago

https://github.com/MongoEngine/mongoengine/pull/2521 updated to return Decimal.

bagerard commented 1 year ago

implementation was merged, will be released in 0.26.0

MongoEngine / mongoengine