New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

regression from 6.0.1 to 6.0.2 / numeric / executemany / setinputsizes: "expecting float" #75

Closed
zzzeek opened this Issue Aug 31, 2017 · 10 comments

Comments

Projects
None yet
2 participants
@zzzeek
Copy link

zzzeek commented Aug 31, 2017

the changes related to numeric have broken existing behavior for executemany():

import cx_Oracle
import decimal

conn = cx_Oracle.connect(
    user="scott",
    password="tiger",
    dsn=cx_Oracle.makedsn(
        "oracle1120", 1521, sid="xe",
    )
)

cursor = conn.cursor()


cursor.execute("""
CREATE TABLE t (
    x NUMERIC(10, 2)
)
""")
try:
    # works
    cursor.setinputsizes(x=cx_Oracle.NUMBER)
    cursor.execute(
        "INSERT INTO t (x) VALUES (:x)",
        x=decimal.Decimal("25.8")
    )

    # fails: "expecting float"
    cursor.setinputsizes(x=cx_Oracle.NUMBER)
    cursor.executemany(
        "INSERT INTO t (x) VALUES (:x)",
       [{"x": decimal.Decimal("25.8")}, {"x": decimal.Decimal("30.0")}]
    )

finally:
    cursor = conn.cursor()
    cursor.execute("DROP TABLE t")
    cursor.close()

with cx_oracle 6.0.2:

Traceback (most recent call last):
File "test.py", line 32, in
[{"x": decimal.Decimal("25.8")}, {"x": decimal.Decimal("30.0")}]
TypeError: expecting float

succeeds with oracle 6.0.1

@anthony-tuininga

This comment has been minimized.

Copy link
Member

anthony-tuininga commented Aug 31, 2017

Yes, this is expected as a result of correcting issue #68. The call to execute() succeeds because the initial failure is swallowed and a new bind variable is created -- or in other words the setinputsizes() call is effectively ignored. For executemany(), however, that can't be done. For the scenario noted above, you can skip the call to setinputsizes() completely or you can use this instead to specify that you want to use decimal.Decimal.

cursor.setinputsizes(x=decimal.Decimal)

If I simply coerce to float, then issue #68 will reappear. :-)

One other possibility is to do a similar thing to execute() -- namely, if the setinputsizes() call is invalid, ignore it and create the variable based on the input data instead. Any thoughts on that?

@zzzeek

This comment has been minimized.

Copy link
Author

zzzeek commented Aug 31, 2017

So as you know I'm just in the middle here, my users have SQLAlchemy installed and so far we had it so that when they upgrade cx_Oracle from 5.x to 6, first due to #68 they'd lose precision, and now on a minor point release numeric support breaks entirely.

I can push out new SQLAlchemy's but changing the setinputsizes behavior here, if I do that in older SQLAlchemy versions, now is that going to break people that are pinned on cx_Oracle 5.x or not. This breaks-in-a-new-way-each-time is creating a lot of release work for me and as far as bigger setinputsizes changes I'd really rather not have to backport that if possible.

@zzzeek

This comment has been minimized.

Copy link
Author

zzzeek commented Aug 31, 2017

One other possibility is to do a similar thing to execute() -- namely, if the setinputsizes() call is invalid, ignore it and create the variable based on the input data instead. Any thoughts on that?

I would favor this if it means a loss in performance until I can get the correct improvements ported downstream, graceful degradation is always preferred.

@anthony-tuininga

This comment has been minimized.

Copy link
Member

anthony-tuininga commented Aug 31, 2017

I do understand and am really not trying to make your life more difficult! Please keep reporting issues and I'll do my best to resolve them for you. Its the only way I can figure out other people's requirements! Further work is being done on the test suite so that future issues like these won't take place, too.

I just checked and cursor.setinputsizes(decimal.Decimal) is not supported in cx_Oracle 5.x. If you avoid the call to setinputsizes() it works fine, but the direct call to cursor.setinputsizes() doesn't work, unfortunately. I could add that support as it is very simple to do, but that wouldn't help you out very much so I'll just leave that alone.

I'll look into what changes would be required to support the graceful degradation (ie cursor.setinputsizes() is ignored) and report back later.

@zzzeek

This comment has been minimized.

Copy link
Author

zzzeek commented Aug 31, 2017

OK that's fine, I'm in an intense period of getting SQLAlchemy 1.2 going so I can likely modernize setinputsizes() there, your earlier suggestion that I just don't need to call it at all for numerics is likely the best for me, that will work on 5.x also ?

@zzzeek

This comment has been minimized.

Copy link
Author

zzzeek commented Aug 31, 2017

also thanks a ton for moving to github and having great responses here. I've been working with cx_Oracle for years and it's only now that I finally get to have a dialogue with you :)

@anthony-tuininga

This comment has been minimized.

Copy link
Member

anthony-tuininga commented Aug 31, 2017

OK that's fine, I'm in an intense period of getting SQLAlchemy 1.2 going so I can likely modernize setinputsizes() there, your earlier suggestion that I just don't need to call it at all for numerics is likely the best for me, that will work on 5.x also ?

Yes, that should work just fine.

also thanks a ton for moving to github and having great responses here. I've been working with cx_Oracle for years and it's only now that I finally get to have a dialogue with you :)

You're very welcome. It was a good move all around. :-)

zzzeek added a commit to zzzeek/sqlalchemy that referenced this issue Aug 31, 2017

- pin on cx_Oracle 6.0.1 for the moment while we wait for
either oracle/python-cx_Oracle#75
to be fixed or we can merge a workaround

Change-Id: Ia3927337fb48824e0fdc764ed3a9d4930ca7a9c6

zzzeek added a commit to zzzeek/sqlalchemy that referenced this issue Aug 31, 2017

- pin on cx_Oracle 6.0.1 for the moment while we wait for
either oracle/python-cx_Oracle#75
to be fixed or we can merge a workaround

Change-Id: Ia3927337fb48824e0fdc764ed3a9d4930ca7a9c6
(cherry picked from commit de73c6d)

anthony-tuininga added a commit that referenced this issue Sep 1, 2017

Ensure that a call to setinputsizes() with an invalid type prior to a…
… call to

executemany() does not result in a type error, but instead gracefully ignores
the call to setinputsizes() as required by the DB API
(#75).

anthony-tuininga added a commit that referenced this issue Sep 1, 2017

Ensure that a call to setinputsizes() with an invalid type prior to a…
… call to

executemany() does not result in a type error, but instead gracefully ignores
the call to setinputsizes() as required by the DB API
(#75).

@anthony-tuininga anthony-tuininga added the bug label Sep 1, 2017

@anthony-tuininga

This comment has been minimized.

Copy link
Member

anthony-tuininga commented Sep 1, 2017

I have added a commit to master and the v6.0.x branch (what will become 6.0.3) that addresses this issue. Specifically, if a call to setinputsizes() is made with the wrong type prior to calling executemany(), no attempt is made to create a new variable like with execute(). This issue is present in cx_Oracle 5.x as well. Just change your code to say cursor.setinputsizes(x=str) to see what I mean!

@zzzeek

This comment has been minimized.

Copy link
Author

zzzeek commented Sep 1, 2017

oh definitely, I had to disallow putting string types in setinputsizes from the beginning, things broke.

for 1.2 I'm going with nothing going into setinputsizes except the LOB types.

@anthony-tuininga

This comment has been minimized.

Copy link
Member

anthony-tuininga commented Sep 1, 2017

for 1.2 I'm going with nothing going into setinputsizes except the LOB types.

Sounds good. Note that passing a long string (or long bytes) when executing insert/update statements can be considerably faster than using LOBs if you pass the entire value at once. I'm not sure whether that is relevant for you so just giving you that factoid and you can do with it what you want! :-)

zzzeek added a commit to zzzeek/sqlalchemy that referenced this issue Sep 26, 2017

- pin on cx_Oracle 6.0.1 for the moment while we wait for
either oracle/python-cx_Oracle#75
to be fixed or we can merge a workaround

Change-Id: Ia3927337fb48824e0fdc764ed3a9d4930ca7a9c6
(cherry picked from commit de73c6d)
(cherry picked from commit 89fab28)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment