cx_Oracle 7.0 regression, param.getvalue() returning data as a list

[zzzeek]

cx_Oracle 7.0 has changed the behavior of a parameter in the context of RETURNING in a breaking way, which breaks SQLAlchemy's support for RETURNING.

test case:

import cx_Oracle

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

cursor = conn.cursor()

try:
    cursor.execute("drop sequence my_seq")
except:
    pass

try:
    cursor.execute("drop table my_table")
except:
    pass

cursor.execute("create sequence my_seq")
cursor.execute("create table my_table (id integer primary key, data varchar(10))")

param = cursor.var(
    cx_Oracle.STRING,
    255,
    arraysize=cursor.arraysize,
    outconverter=int
)

cursor.execute(
    "insert into my_table(id, data) "
    "values (my_seq.nextval, :data) "
    "RETURNING my_table.id INTO :ret_0",
    {"data": "mydata", "ret_0": param}
)

val = param.getvalue()

# passes < cx_Oracle 7.0, cx_Oracle == 7.0 gets:
# AssertionError: [1]
assert val == 1, val

[anthony-tuininga]

@zzzeek, yes, this is intended! See your comment on this issue. cx_Oracle 6.3 and 6.4 can use the 7.0 behaviour by enabling cx_Oracle.future.dml_ret_array_val but earlier versions cannot. I'm afraid you'll have to check the version and adjust your code accordingly.

If you only ever care about a single value being returned, then you can simply check the result and if it is an array, check its length (and return an error if it exceeds one) and otherwise return the first element of the array.

This demonstrates the issue with multiple rows being returned, however. If your table has the data column increased to size 100, then this is what you would see with the following code in cx_Oracle 7:

data = [(1, 'Banana'), (2, 'Watermelon'), (3, 'Kiwi'), (4, 'Apple')]
cursor.executemany("insert into my_table values (:1, :2)", data)
cursor.execute("""
        update my_table set
            data = data || ' (Accepted)'
        where instr(data, 'a') != 0
        returning id into :ret_0""",
        ret_0 = param)

print("Result:", param.getvalue())
# yields the result [1, 2]
# before it would have yielded the result 1

If you do this sort of thing with executemany(), then you get an array of arrays -- which wasn't possible prior to cx_Oracle 6.3.

[zzzeek]

is var.values available? the check that you refer towards is a bit wasteful.

[zzzeek]

Can I presume that if __future__ is present on cx_Oracle, then dml_ret_array_val is present and I can rely upon that ?

[zzzeek]

seems to fail under 6.2.1:

cx_Oracle.future.dml_ret_array_val = True

then:

val = param.values

assert val[0:5] == [[1], [], [], [], []]

this fails, the dml_ret_array_val flag is present on __future__ but does not seem to work.

So I need to hardcode a version based check for cx_Oracle >= 6.3 , confirm?

[zzzeek]

that is, I have this:

            self._values_are_lists = self.cx_oracle_ver >= (6, 3)
            if self._values_are_lists:
                cx_Oracle.__future__.dml_ret_array_val = True
                self._paramval = lambda value: value.values[0][0]
            else:
                self._paramval = lambda value: value.getvalue()

[anthony-tuininga]

is var.values available? the check that you refer towards is a bit wasteful.

It is available, but I don't think it will help you. In cx_Oracle 7 it would return [[1]] but in cx_Oracle 6 it would return [1]. For the update example I gave it would return [[1, 2]] but in cx_Oracle 6 it would return [1, 2]. The only time you would get multiple elements in the array returned by var.values would be if you were performing executemany().

Can I presume that if __future__ is present on cx_Oracle, then dml_ret_array_val is present and I can rely upon that ?

No. cx_Oracle.__future__ is available in cx_Oracle 6.3 and higher, but the attribute dml_ret_array_val is only relevant in cx_Oracle 6.3 and 6.4. It will be ignored in cx_Oracle 7.

that is, I have this:

            self._values_are_lists = self.cx_oracle_ver >= (6, 3)
            if self._values_are_lists:
                cx_Oracle.__future__.dml_ret_array_val = True
                self._paramval = lambda value: value.values[0][0]
            else:
                self._paramval = lambda value: value.getvalue()

That seems reasonable.

[anthony-tuininga]

But it won't handle update statements returning multiple rows. I'm not sure if that is of concern to you, though.

[zzzeek]

dml_ret_array_val is working in 6.3 and forward so I can use the above approach for 6.3 and up. RETURNING for multiple rows has never been supported for the cx_Oracle dialect so that can be enabled at some point, also no other DBAPI supports correct RETURNING with executemany(), so that's not something SQLAlchemy supports anyway.

[zzzeek]

so just to sum up. param.getvalue() returns a list in all cases now. Wouldn't it have been better to name it param.getvalues() ? then have getvalue() continue to return a scalar, and raise an error if more than one value is available.

[anthony-tuininga]

so just to sum up. param.getvalue() returns a list in all cases now. Wouldn't it have been better to name it param.getvalues() ? then have getvalue() continue to return a scalar, and raise an error if more than one value is available.

Well, param.getvalue() returns a list in all cases now ONLY if it is bound to a DML returning statement (or was created by calling cursor.arrayvar()). If the variable is bound to a regular statement, however, it returns a scalar.

Calling it getvalues() would have been confusing, too, since there are (possibly) multiple rows being bound (as in executemany) and (possibly) multiple rows being returned (as in DML returning) and it wouldn't be clear which one is being referred to. So I think this is reasonable.

[zzzeek]

Well, param.getvalue() returns a list in all cases now ONLY if it is bound to a DML returning statement (or was created by calling cursor.arrayvar()). If the variable is bound to a regular statement, however, it returns a scalar.

OK, hitting this, need to do the decision tree again:

statement has RETURNING, and at least one row is returned, means param.values[0][0] definitiely exists

statement does NOT have RETURNING, is a plain outparam and is not cursor.arrayvar(), means param.values will be a scalar.

how does RETURNING change this? still unclear, if arrayvar() is what means "return a list", are you scanning the statement for the word RETURNING or something?

[zzzeek]

here's the adjusted logic

            self._paramval = lambda value: value.getvalue()
            self._values_are_lists = self.cx_oracle_ver >= (6, 3)
            if self._values_are_lists:
                cx_Oracle.__future__.dml_ret_array_val = True

                def _returningval(value):
                    try:
                        return value.values[0][0]
                    except IndexError:
                        return None

                self._returningval = _returningval
            else:
                self._returningval = self._paramval

if I know the parameter is from a RETURNING, I use _returningval, else for a regular user defined OUT param I continue to use getvalue().

[anthony-tuininga]

how does RETURNING change this? still unclear, if arrayvar() is what means "return a list", are you scanning the statement for the word RETURNING or something?

Oracle provides an attribute that tells you whether or not a DML returning statement has been executed, which I make use of in ODPI-C, the library that cx_Oracle depends on. arrayvar() is intended for PL/SQL arrays (aka index-by tables).

Where is the logic you are changing found in the sqlalchemy code? Is it found here (in cx_oracle.py)?

    def get_result_proxy(self):
        if self.out_parameters and self.compiled.returning:
            returning_params = [
                self.out_parameters["ret_%d" % i].getvalue()
                for i in range(len(self.out_parameters))
            ]
            return ReturningResultProxy(self, returning_params)
# rest of the function is for non-DML returning statements

As long as you are only doing this for the case where you are dealing with DML returning statements it should be fine.

[zzzeek]

OK RETURNING has a magic flag on your end then, that makes more sense

[zzzeek]

well at least nothing else seems to fail on 7.0. can't wait for 8.0! :)

[cjbj]

@zzzeek how can we make it easier for you to test the master branch?

[zzzeek]

@cjbj at the moment I run my CI on my own jenkins server so I would need a job that kicks off when commits come in to cx_Oracle and then builds against latest master. I'd have to figure out how to play with tox.ini and such to get it to install from a git tree rather than from pypi. I already have builds that do the latest 5.x, 6.x and now 7.x pypi releases (which is a lot).

or the other way around would be if cx_Oracle were on travis or something, a nightly SQLAlchemy build could be added to that, but again SQLAlchemy has multiple release streams at the same time also (right now rel_1_2 and master).

[cjbj]

@zzzeek last time I checked, Travis wasn't going to be straightforward for us, for a couple of reasons. :(

Future compatibility decisions in cx_Oracle will continue to be taken carefully, of course.

Thanks for all your work with SQLAlchemy.

[zzzeek]

id have to work out how to trigger builds from the master branches of various DBAPIs and make use of those git masters rather than the pypi build within the pip step. the first part is easy enough with jenkins.

[zzzeek]

I've come up with a simple way to do this for all the DBAPIs, just add "-c reqs.txt" that has the github urls in it for each DBAPI. ill have it run once daily for all masters

[anthony-tuininga]

Nice!

[cjbj]

@zzzeek excellent.