Can't roundtrip json encode/prase properly with ConstrainedDecimal

[Hultner]

Checks

• I added a descriptive title to this issue
• I have searched (google, github) for similar issues and couldn't find anything
• I have read and followed the docs and still think this is a bug

Bug

Output of python -c "import pydantic.utils; print(pydantic.utils.version_info())":

             pydantic version: 1.7.3
            pydantic compiled: True
                 install path: /Users/hultner/Library/Caches/pypoetry/virtualenvs/hantera-backend-5YPPF0bI-py3.9/lib/python3.9/site-packages/pydantic
               python version: 3.9.0 (default, Oct 10 2020, 11:40:52)  [Clang 11.0.3 (clang-1103.0.32.62)]
                     platform: macOS-10.15.7-x86_64-i386-64bit
     optional deps. installed: ['typing-extensions', 'email-validator', 'devtools']

When using a ConstrainedDecimal with zero decimal_places (as provided in example below), pydantic incorrectly encodes the value as a float, thus resulting in failure if one tries to parse the recently encoded json object.

Using a decimal with max_digits = x and decimal_places = 0 is a great way of representing for instance a Numeric(22,0) (where x is 22) from many SQL database schemas. Certain database engines like the popular pyodbc will properly handle and convert such a Decimal value, but won't handle it as an int as this is implicitly interpreted as a 32 bit int by pyodbc. Having a fixed number of max_digits also allows ones query-engine to pre-compile reusable query plans, which other wise would have to be recomputed for every length of of the given number.
In other words, using a ConstrainedDecimal for this type of data is ideal.

I have provided a minimal test/example which can both be executed directly but also ran with pytest to showcase the issue at hand.

"""
Self contained example showcasing problem with decimals using pydantics
default encoder. 
"""
from pydantic import ConstrainedDecimal, BaseModel
from decimal import Decimal


class Id(ConstrainedDecimal):
    max_digits = 22
    decimal_places = 0
    ge = 0


ObjId = Id


class Obj(BaseModel):
    id: ObjId
    name: str
    price: Decimal = Decimal(0.00)


class ObjWithEncoder(BaseModel):
    id: ObjId
    name: str
    price: Decimal = Decimal(0.00)

    class Config:
        json_encoders = {
            Id: int,
        }


def test_con_decimal_encode() -> None:
    test_obj = Obj(id=1, name="Test Obj")
    cycled_obj = Obj.parse_raw(test_obj.json())
    assert test_obj == cycled_obj


def test_con_decimal_encode_custom_encoder() -> None:
    test_obj = ObjWithEncoder(id=1, name="Test Obj")
    cycled_obj = ObjWithEncoder.parse_raw(test_obj.json())
    assert test_obj == cycled_obj


if __name__ == "__main__":
    from pprint import pprint

    print_obj = Obj(id=1, name="Test Obj", price=1.23)
    json_obj = print_obj.json()
    pprint(json_obj)

I have a small patch for pydantic.json-module which I can provide as a pull-request. When using said patch all the tests above will pass, notice that I both handle the case where Decimals do have an negative exponent (decimal_places) and the case where it doesn't. The patch handles both these cases as expected and is written in a minimal, and of course readable fashion.

I am right now in the process of reading through the contributor guidelines to ensure that my patch is up to the standards which the project holds for contributions.

[Hultner]

Patch provided in #2294

[Hultner]

@samuelcolvin It's great that the fix for the bug in particular got merged, but thinking further about it.

Isn't it a bit strange that I still can't provide a custom encoder for the Id type? It would make sense if the encoder I provided would be used. I suspect this is because the actual type checked when encoding is that of the instance and not the type defined in the objects model class. Wouldn't it make sense to check the annotated type first for a custom encoder?
And if not I think it would make sense to have this limitation clearly documented.
Of course either way this would then be a new separate (though related) issue as we've already resolved the particular problem with decimals.