Behavior for Numeric.asdecimal=False is different than expected in many dialects

Issue #1624 resolved
Former user created an issue

(original reporter: ged) Based on the docstring, and the "base Numeric" code (in types.py), one would think that if "asdecimal" is False, the value is passed unchanged from the DBAPI. In many dialects, this is not true as their result_processor actively converts from Decimal to float.

As a side note, Oracle's dialects specific types seem to use a default value of None, and computes its value dynamically.

It might be a good idea to change the meaning of asdecimal: False would mean "actively convert" and None would mean "pass through".

Comments (8)

  1. Mike Bayer repo owner
    • changed milestone to 0.6.0

    False/True should conform to float/Decimal completely, either through an active conversion or knowledge of the underlying DBAPI's behavior. So what's the use case for "None" ? Decimal/float objects are entirely incompatible with each other - you have to explicitly expect one or the other. If you're looking for performance, the Numeric type ideally wouldn't convert for a dialect that already returns the expected type (such as psycopg2/pg8000 returning Decimal already, for example).

  2. Former user Account Deleted

    (original author: ged) Quote from the docstring of Numeric:

           :param asdecimal: default True.  If False, values will be
              returned as-is from the DB-API, and may be either
              ``Decimal`` or ``float`` types depending on the DB-API in
              use.
    

    Excerpt from the code of types.py/Numeric (which does pass through values, as stated in the docstring):

    def result_processor(self, dialect, coltype):
        if self.asdecimal:
            ...
        else:
            return None
    

    Excerpt from dialects/postgresql/psycopg2.py (similar code is found in at least 5 other dialects)

    elif coltype == 1700:
        def process(value):
            if value is not None:
                return float(value)
            else:
                return value
        return process
    

    I hope you see the problem now, because I don't know what else I could say to explain it...

  3. Mike Bayer repo owner

    Replying to ged:

    Quote from the docstring of Numeric: {{{ :param asdecimal: default True. If False, values will be returned as-is from the DB-API, and may be either Decimal or float types depending on the DB-API in use. }}}

    eyes pop. That docstring is plainly incorrect. Even in 0.5, PGNumeric coaxes Decimals into float when the flag is false. It should be fixed in 0.5 and 0.6.

    Excerpt from the code of types.py/Numeric (which does pass through values, as stated in the docstring): {{{ #!python def result_processor(self, dialect, coltype): if self.asdecimal: ... else: return None }}}

    Excerpt from dialects/postgresql/psycopg2.py (similar code is found in at least 5 other dialects) {{{ #!python elif coltype == 1700: def process(value): if value is not None: return float(value) else: return value return process }}}

    I hope you see the problem now, because I don't know what else I could say to explain it...

    from my view the docs are wrong. So...again a documentation issue ? I don't mind much if we add None as a "passthrough guaranteed" mode but I doubt its actually useful to anyone (assuming we ensure that "passthrough" is used as often as possible in conjunction with the known return type of the DBAPI to minimize overhead).

  4. Former user Account Deleted

    (original author: ged) Replying to zzzeek:

    Replying to ged:

    Quote from the docstring of Numeric: {{{ :param asdecimal: default True. If False, values will be returned as-is from the DB-API, and may be either Decimal or float types depending on the DB-API in use. }}}

    eyes pop. That docstring is plainly incorrect. Even in 0.5, PGNumeric coaxes Decimals into float when the flag is false. It should be fixed in 0.5 and 0.6.

    Excerpt from the code of types.py/Numeric (which does pass through values, as stated in the docstring): {{{ #!python def result_processor(self, dialect, coltype): if self.asdecimal: ... else: return None }}}

    from my view the docs are wrong. So...again a documentation issue ?

    Not only. There is also the "default" implementation in types.py/Numeric (quoted above) which does pass values through if asdecimal is False.

    I don't mind much if we add None as a "passthrough guaranteed" mode but I doubt its actually useful to anyone (assuming we ensure that "passthrough" is used as often as possible in conjunction with the known return type of the DBAPI to minimize overhead).

    None as passthrough proposition is only useful (IMO) as the default value, which would have forced users to choose what kind of data they want, and make them aware of any conversion process either way since they would actively ask for it. Since you don't want to change the default value (which I perfectly understand), a forced passthrough is useless.

  5. Mike Bayer repo owner

    Replying to ged:

    Not only. There is also the "default" implementation in types.py/Numeric (quoted above) which does pass values through if asdecimal is False.

    no, that's based on the assumption that all DBAPIs which have not provided their own Numeric type return floats by default. Only the PG dialects are the ones that I've seen which return Decimals. the contract is firmly False=float, True=Decimal.

    I don't mind much if we add None as a "passthrough guaranteed" mode but I doubt its actually useful to anyone (assuming we ensure that "passthrough" is used as often as possible in conjunction with the known return type of the DBAPI to minimize overhead).

    None as passthrough proposition is only useful (IMO) as the default value, which would have forced users to choose what kind of data they want, and make them aware of any conversion process either way since they would actively ask for it.

    They will leave it as None not knowing about it, then port their app from MySQL to PG, and their app will break. Then they will complain that its a bug in SQLAlchemy. So no, this has to be set at one or the other. If there's one behavior users would like to be implicit, its that their applications are as portable as possible to other databases without them having to hunt down options to make it such. This is not possible in all cases but here is one where it is - I vote to leave it as True for Numeric and False for Float.

  6. Former user Account Deleted

    (original author: ged) Replying to zzzeek:

    no, that's based on the assumption that all DBAPIs which have not provided their own Numeric type return floats by default.

    Makes sense.

    They will leave it as None not knowing about it, then port their app from MySQL to PG, and their app will break. Then they will complain that its a bug in SQLAlchemy.

    Hehe, yeah, that's seem very plausible indeed.

    So no, this has to be set at one or the other. If there's one behavior users would like to be implicit, its that their applications are as portable as possible to other databases without them having to hunt down options to make it such.

    This is where the problem is. Their applications will seem to be portable but in reality they are subtly not.

    This is not possible in all cases but here is one where it is - I vote to leave it as True for Numeric and False for Float.

    Yes, it's probably the best option. But I'm still convinced a warning would be a good idea.

  7. Mike Bayer repo owner

    docs are updated and published in the rst in a2c727788e6746b58455c2c4cbd584043b66077d. Since you can't really conveniently write your app to expect "both" kinds of values, and you have to choose, a warning IMHO would be an annoyance to an app that has chosen one type or another across many dialects.

  8. Log in to comment