Node-oracledb: Using the v4.0.0, when the `Object.prototype` had been chained a method, the `Connection.executeMany` goes error with `NJS-060` prompt.

[TheNorthMemory]

The behavior is similar to node-modules/urllib#312, the sample code as below:

const data = [
    {"rate_user":"Bob","rate_time":"2019-07-26 19:42:36","item_price":338,"rating":"I like it"},
    {"rate_user":"Alice","rate_time":"2019-07-26 19:42:36","item_price":338,"rating":"I like it too"}
];

const cfg = require('./dbConfig');
const oracledb = require('oracledb');

Object.prototype.noop = () => {}

(async () => {

  const options = {
    autoCommit: true,
    bindDefs: {
      item_price : { type: oracledb.NUMBER },
      rate_time  : { type: oracledb.STRING, maxSize: 32 },
      rate_user  : { type: oracledb.STRING, maxSize: 128 },
      rating     : { type: oracledb.STRING, maxSize: 4000 },
    }
  };

  const sql = `MERGE INTO RATING R USING ( SELECT
      :item_price as itemPrice,
      :rate_time as rateTime,
      :rate_user as rateUser,
      :rating as rating
  FROM DUAL ) T ON (
      R.RATE_TIME     = T.rateTime
      AND R.RATE_USER = T.rateUser
  ) WHEN MATCHED THEN UPDATE SET
      R.ITEM_PRICE = T.itemPrice
      , R.RATING   = T.rating
  WHEN NOT MATCHED THEN INSERT (
      RATE_TIME
      , RATE_USER
      , ITEM_PRICE
      , RATING
  ) VALUES (
      T.rateTime
      , T.rateUser
      , T.itemPrice
      , T.rating
  )`;

  const conn = await oracledb.getConnection(cfg);
  try {
    await conn.executeMany(sql, data, options);
  } catch (e) {
    console.error(e);
  }
  await conn.close();

})();

1. Is it an error or a hang or a crash?

crashed with this message:Error: NJS-060: type must be specified for bind "noop"

1. What error(s) you are seeing?

Error: NJS-060: type must be specified for bind "noop"
    at Connection.executeMany (/Users/james/data.git/node_modules/oracledb/lib/connection.js:207:21)
    at /Users/james/data.git/node_modules/oracledb/lib/util.js:180:16
    at new Promise (<anonymous>)
    at Connection.executeMany (/Users/james/data.git/node_modules/oracledb/lib/util.js:168:14)
    at /Users/james/data.git/test/rate.executeMany.test.js:48:18

1. Run node and show the output of:

Welcome to Node.js v12.6.0.
Type ".help" for more information.
> process.platform
'darwin'
>
> process.version
'v12.6.0'
> process.arch
'x64'
> require('oracledb').versionString
'4.0.0'
>

[cjbj]

Thanks for the report. Keep testing and let us know what else you find.

Did this work in in node-oracledb 3.1.2?

Can you share the SQL to create the table?

[TheNorthMemory]

it works well under [email protected] and [email protected].

table script as below:

CREATE TABLE "DATAHOUSE"."RATING" 
   (    "RATE_TIME" VARCHAR2(32), 
    "RATE_USER" VARCHAR2(128), 
    "ITEM_PRICE" NUMBER, 
    "RATING" VARCHAR2(4000)
   )

[cjbj]

Thank you. We'll review it.

[anthony-tuininga]

In discussion with @dmcghan I have confirmed that the v3 code used a method that only returned its "own" property names. v4 uses N-API which does not provide that directly, so some additional effort will need to be made to filter out properties that are not "owned" by the object. A patch will follow in due time. Thanks for letting us know about this!

[anthony-tuininga]

Here is a preliminary patch:

diff --git a/src/njsBaton.c b/src/njsBaton.c
index 3137446d..699bb7fb 100644
--- a/src/njsBaton.c
+++ b/src/njsBaton.c
@@ -452,7 +452,8 @@ bool njsBaton_getFetchInfoFromArg(njsBaton *baton, napi_env env,
         return true;

     // extract the property names from the object
-    NJS_CHECK_NAPI(env, napi_get_property_names(env, value, &keys))
+    if (!njsUtils_getOwnPropertyNames(env, value, &keys))
+        return false;

     // allocate space for the fetchInfo based on the number of keys
     NJS_CHECK_NAPI(env, napi_get_array_length(env, keys, &numElements))
diff --git a/src/njsConnection.c b/src/njsConnection.c
index 6826ee02..5e2d6488 100644
--- a/src/njsConnection.c
+++ b/src/njsConnection.c
@@ -1982,9 +1982,8 @@ static bool njsConnection_processExecuteBinds(njsBaton *baton,

     // if binding by name, get the list of bind names
     bindNames = NULL;
-    if (!isArray) {
-        NJS_CHECK_NAPI(env, napi_get_property_names(env, binds, &bindNames))
-    }
+    if (!isArray && !njsUtils_getOwnPropertyNames(env, binds, &bindNames))
+        return false;

     // initialize variables; if there are no variables, nothing further to do!
     baton->bindArraySize = 1;
@@ -2057,9 +2056,8 @@ static bool njsConnection_processExecuteManyBinds(njsBaton *baton,

     // get the list of bind names, if binding by name
     bindNames = NULL;
-    if (!bindByPos) {
-        NJS_CHECK_NAPI(env, napi_get_property_names(env, bindDefs, &bindNames))
-    }
+    if (!bindByPos && !njsUtils_getOwnPropertyNames(env, bindDefs, &bindNames))
+        return false;

     // initialize variables; if there are no variables, nothing further to do!
     if (!njsConnection_initBindVars(baton, env, bindDefs, bindNames))
diff --git a/src/njsModule.h b/src/njsModule.h
index d712baf2..f8867b6a 100644
--- a/src/njsModule.h
+++ b/src/njsModule.h
@@ -943,6 +943,8 @@ bool njsUtils_getError(napi_env env, dpiErrorInfo *errorInfo,
 bool njsUtils_getIntArg(napi_env env, napi_value *args, int index,
         int32_t *result);
 napi_value njsUtils_getNull(napi_env env);
+bool njsUtils_getOwnPropertyNames(napi_env env, napi_value value,
+        napi_value *names);
 bool njsUtils_getStringArg(napi_env env, napi_value *args, int index,
         char **result, size_t *resultLength);
 bool njsUtils_getStringFromArg(napi_env env, napi_value *args,
diff --git a/src/njsUtils.c b/src/njsUtils.c
index ea8b57b8..0a7755c0 100644
--- a/src/njsUtils.c
+++ b/src/njsUtils.c
@@ -508,6 +508,27 @@ napi_value njsUtils_getNull(napi_env env)
 }


+//-----------------------------------------------------------------------------
+// njsUtils_getOwnPropertyNames()
+//   Returns an array of property names owned specifically by the given object.
+//-----------------------------------------------------------------------------
+bool njsUtils_getOwnPropertyNames(napi_env env, napi_value value,
+        napi_value *names)
+{
+    napi_value global, globalObject, method;
+
+    NJS_CHECK_NAPI(env, napi_get_global(env, &global))
+    NJS_CHECK_NAPI(env, napi_get_named_property(env, global, "Object",
+            &globalObject))
+    NJS_CHECK_NAPI(env, napi_get_named_property(env, globalObject,
+            "getOwnPropertyNames", &method))
+    NJS_CHECK_NAPI(env, napi_call_function(env, globalObject, method, 1,
+            &value, names))
+
+    return true;
+}
+
+
 //-----------------------------------------------------------------------------
 // njsUtils_getStringArg()
 //   Gets a string from the specified parameter. If the value is not a string,

[cjbj]

@TheNorthMemory have you tested the proposed patch? We'd like confirmation before we merge it. You will need to build the source code with the patch applied: https://oracle.github.io/node-oracledb/INSTALL.html#github

[TheNorthMemory]

@cjbj before we were only fetch the package from npmjs.com onto the docker container. Let me see how to build from the source code with this patch. feedback later.

[cjbj]

@TheNorthMemory thank you.

Are there any other issues you're seeing with 4.0.0?

[TheNorthMemory]

@cjbj @anthony-tuininga great thx the patch, it works well.

just a little bit problem that hard to build the custom package :(.

here's my env & steps:

uname -a && env

Linux e783b2147f67 3.10.0-862.9.1.el7.x86_64 #1 SMP Mon Jul 16 16:29:36 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux

NODE_VERSION=12.7.0
HOSTNAME=e783b2147f67
LD_LIBRARY_PATH=/usr/lib/oracle/11.2/client64/lib
PATH=/usr/lib/oracle/11.2/client64/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
PWD=/app
LANG=en_US.UTF-8
SHLVL=1
HOME=/root
YARN_VERSION=1.17.3

mkdir -p /app
yum install -y gcc-c++ make
npm install node-gyp --verbose -g
git clone -b v4.0.0 --depth 1 --single-branch https://github.com/oracle/node-oracledb.git /app/node-oracledb
cd /app/node-oracledb
git submodule init
git submodule update
git apply /tmp/v4.0.0.patch
cd /app
npm install /app/node-oracledb

and then, i'd run the first floor javascript, it works.

i'd also try to build a fresh npm-style-package, unfortunately, i didnot got the package, steps as below:

cd /app/node-oracledb
node-gyp clean
npm build .

npm WARN lifecycle [email protected]~install: cannot run in wd [email protected] node-gyp rebuild (wd=/app/node-oracledb)

and retried following steps, also failed:

cd /app/node-oracledb
node-gyp clean
node-gyp rebuild
npm run buildbinary

> [email protected] buildbinary /app/node-oracledb
> node package/buildbinary.js

Building binary oracledb-4.0.0-linux-x64.node using Node.js v12.7.0
npm WARN lifecycle [email protected]~install: cannot run in wd [email protected] node-gyp rebuild (wd=/app/node-oracledb)
ENOENT: no such file or directory, rename 'build/Release/oracledb.node' -> 'package/Staging/oracledb-4.0.0-linux-x64.node'

i think that the patch solution is only verified work, but not product ready, waiting for the official npm package.

[cjbj]

@TheNorthMemory thanks for the confirmation and detail.

I'm not totally surprised that buildbinary doesn't work with the Node rpms. Those packages are a little different to the binaries I am using from nodejs.org. It would be great if you submit a PR to make it work.

PS, Oracle Client 19.3 connects to Oracle DB 11.2 or later and can be installed from yum.oracle.com without needing a clickthrough, e.g.: yum -y install oracle-release-el7 && yum -y install oracle-instantclient19.3-basic

[TheNorthMemory]

@cjbj patching here

diff --git a/package/buildbinary.js b/package/buildbinary.js
index 5d6e9ce6..010be3d7 100644
--- a/package/buildbinary.js
+++ b/package/buildbinary.js
@@ -66,7 +66,7 @@ function buildBinary() {
     fs.unlink(buildBinaryFile, function(err) {if (err && !err.message.match(/ENOENT/)) throw(err);});
     fs.unlink(binaryStagingFile, function(err) {if (err && !err.message.match(/ENOENT/)) throw(err);});
     fs.unlink(binaryStagingInfoFile, function(err) {if (err && !err.message.match(/ENOENT/)) throw(err);});
-    execSync('npm install');
+    execSync('node-gyp rebuild');
     fs.renameSync(buildBinaryFile, binaryStagingFile);
     fs.appendFileSync(binaryStagingInfoFile, buildInfo + "\n");
   } catch(err) {

and then, the full working steps as below:

mkdir -p /app
yum install -y gcc-c++ make
npm install node-gyp --verbose -g
git clone -b v4.0.0 --depth 1 --single-branch https://github.com/oracle/node-oracledb.git /app/node-oracledb
cd /app/node-oracledb
git submodule init
git submodule update
git apply /tmp/v4.0.0.patch
npm run buildbinary --verbose
npm run buildpackage --verbose
cd /app
npm install /app/node-oracledb/oracledb-4.0.0.tgz --no-save

tested under mac [email protected]&[email protected] and centos7 [email protected]&[email protected], both are working expectedly.

[cjbj]

That works in my macOS and Linux environments; I'll get Windows checked and see about merging. Thanks!

[cjbj]

@TheNorthMemory node-oracledb 4.0.1 has the fix for adding enumerable properties to
Object.prototype. Thanks for reporting the problem.