oracle / node-oracledb

Oracle Database driver for Node.js maintained by Oracle Corp.
http://oracle.github.io/node-oracledb/
Other
2.26k stars 1.07k forks source link

ORA-24338 when OUT REF CURSOR is not set #886

Closed jdaley closed 4 years ago

jdaley commented 6 years ago

I have a regression after upgrading oracledb from 1.13.1 to 2.1.2, for executing PL/SQL procedures with an OUT REF CURSOR that does not get set.

Example PL/SQL procedure:

procedure testproc(status out varchar2, lines out sys_refcursor)
is
begin
  -- ...code which may or may not set the 'lines' cursor...
  status := 'new';
end testproc;

Example JS:

const result = await connection.execute(
    "BEGIN testpkg.testproc(:0, :1); END;",
    [
        { dir: oracledb.BIND_OUT, type: oracledb.STRING },
        { dir: oracledb.BIND_OUT, type: oracledb.CURSOR }
    ]
);

In version 1.13.1, this returns a result with outBinds: [ 'new', null ]

In version 2.1.2, this throws ORA-24338: statement handle not executed

The version 1 behaviour is preferred because a null out cursor is not necessarily an error (at least, in this database) and it allows you to still read the values of other out parameters.


Node.js version: v8.10.0 (linux x64) Node-oracledb version: 2.1.2 Oracle Client library version: 11.2.0.2.0 Oracle Database version: 11.2.0.2.0 Ubuntu 16.04 in Docker gcc (Ubuntu 5.4.0-6ubuntu1~16.04.9) 5.4.0 20160609 export LD_LIBRARY_PATH=$ORACLE_HOME/lib

cjbj commented 6 years ago

I marked this a bug, but have a vague memory that we require you to set the out bind in all cases due to some OCI thing that can't be worked around. I'll take a look later.

jdaley commented 6 years ago

I did some code spelunking and it looks like version 1 verifies that OCI_ATTR_STMT_STATE is set to OCI_STMT_STATE_EXECUTED, otherwise it returns null. See https://github.com/oracle/node-oracledb/blob/v1.13.1/src/njs/src/njsConnection.cpp#L1938

That seems like a good check. I did some testing inside odpi/src/dpiStmt.c to check this state. If the cursor is set, it will come back as OCI_STMT_STATE_EXECUTED. If not set, or set to null, it will come back as OCI_STMT_STATE_INITIALIZED. But I don't think ODPI-C provides any way to get this state at the moment.

cjbj commented 6 years ago

The current thoughts are that you should execute at least a dummy query and avoid returning an uninitialized REF CURSOR. This will stop problems with node-oracledb 2 and any other client tools accessing the stored procedure. Any future real bug fix would require Oracle DB and Oracle client library patches. The OCI_STMT_STATE_EXECUTED check doesn't seem the proper way. The base problem is related to https://github.com/oracle/node-oracledb/issues/344 but unfortunately setting the cursor to NULL doesn't work. It is being tracked in Oracle bug 22646514.

jdaley commented 6 years ago

The dummy query idea would work but it gets messy to return the right column types and cover every path through the code. The cleanest way is probably to put OPEN xx FOR SELECT * FROM DUAL WHERE 1 = 0 at the top of each procedure to serve as a default value, and then modify client apps to not freak out when the result set doesn't have the column metadata they are expecting. Still a bit messy.. we'll stick with v1 for now.

Other drivers I have on hand are the JDBC thin driver (ojdbc8.jar) and the .NET managed driver. Both of these handle the NULL out cursors fine. They just return null in the out parameter.

I gave the JDBC OCI driver a try, and it is different. For a NULL out cursor, you get a ResultSet object with empty metadata, which throws an error if you try to read from it. The statement still executes successfully, and still gives you back all the other out param values. If node-oracledb v2 can't return a null, would it be possible to match the JDBC OCI behaviour? i.e. error if you try to read the ResultSet, not error for the whole statement?

cjbj commented 6 years ago

@jdaley Last time we discussed this, we concluded we needed the underlying OCI fix. I've regularly been chasing up the OCI developer who has it in his queue, but no update yet.

chrisabovo commented 6 years ago

Hi @cjbj and @jdaley

i have the same problem, but i would not like to revert to node-oracledb@1.13.1

I found an simple workaround to solve this.

Just add if(:o_cursor IS NULL) then OPEN :o_cursor FOR SELECT * FROM dual WHERE 1 = 0; end if; before END; in your SQL:

var sql =
      "BEGIN PRO_TEST(:i_val1, :i_val2, :o_cursor, :o_msg_return); if(:o_cursor IS NULL) then OPEN :o_cursor FOR SELECT * FROM dual WHERE 1 = 0; end if; END;";
    var bindvars = {
      i_val1: 'TEST1',
      i_val2: 'TEST2',
      o_cursor: { type: oracledb.CURSOR, dir: oracledb.BIND_OUT },
      o_msg_return: { type: oracledb.STRING, dir: oracledb.BIND_OUT }
    };
    let options = { };
    connection.execute(sql, bindvars, options, function(err, result) {
      if (err) {
        console.error("Error ==>", err);
        return;
      }
      console.log('Result ==>', result.outBinds['o_retorno']);
    });

This check if return cursor is null and initialize then with empty result.

This temporarily solves the problem.

Thanks!

cjbj commented 6 years ago

@chrisabovo thanks for sharing.

alex-michaud commented 6 years ago

Same problem here, v2.3.0. But I wonder why this problem is not present on version 1. If the problem is at the OCI level but it was working in v1, I presume it's because there was a workaround in v1? If so, could it then be possible to find a temporary workaround for v2?

cjbj commented 6 years ago

@alex-michaud v1 had substantially different code paths.

vrsbrazil commented 5 years ago

We have experienced this issue it seems right after a procedure got recompiled, once we rebooted the database connection, it worked fine. Anyone else experienced this? Any thoughts?

dmcghan commented 5 years ago

@vrsbrazil Please create a new GitHub issue for your problem. Also, please include a reproducible test case (if possible) and explain what you mean by "once we rebooted the database connection". Are you rebooting the DB, opening/closing a connection pool, or doing something else?

cjbj commented 5 years ago

@vrsbrazil the behavior is undefined, so I can understand some randomness. The sane workaround is to do something like https://github.com/oracle/node-oracledb/issues/886#issuecomment-393548265.

vrsbrazil commented 5 years ago

We have experienced this issue it seems right after a procedure got recompiled, once we rebooted the database connection, it worked fine. Anyone else experienced this? Any thoughts?

An issue was opened for this: #1036

cjbj commented 4 years ago

As noted a couple of years ago this problem is in layers beneath node-oracledb. I am continuing to track that with the owning team. Since this isn't specific to node-oracledb, I'm going to close this issue.

cjbj commented 2 years ago

An update: The underlying OCI issue (bug 22813401) has been fixed. The patch will be released in our next major Oracle DB version (whatever comes after the current 21c).

cjbj commented 1 year ago

I can now say the fix is in 23c. It will also land in 19.20.