Improved bind error message NJS-098 with cleaner description that is thrown for all bind mismatch scenarios

Author: sharadraju

doc/src/release_notes.rst

@@ -72,6 +72,9 @@ Thin Mode Changes
 
 #) Provide additional error details when an invalid wallet is used.
 
+#)  Improved bind error message ``NJS-098`` with cleaner description that is
+    thrown for all bind mismatch scenarios.
+
 Thick Mode changes
 ++++++++++++++++++
 

lib/errors.js

@@ -108,7 +108,7 @@ const ERR_MISSING_FILE = 91;
 const ERR_INVALID_NUMBER_OF_CONNECTIONS = 92;
 const ERR_EXEC_MODE_ONLY_FOR_DML = 95;
 const ERR_INVALID_BIND_NAME = 97;
-const ERR_WRONG_NUMBER_OF_POSITIONAL_BINDS = 98;
+const ERR_WRONG_NUMBER_OF_BINDS = 98;
 const ERR_BUFFER_LENGTH_INSUFFICIENT = 99;
 const ERR_NCHAR_CS_NOT_SUPPORTED = 100;
 const ERR_MISSING_CREDENTIALS = 101;
@@ -353,8 +353,8 @@ messages.set(ERR_EXEC_MODE_ONLY_FOR_DML,                // NJS-095
   'setting "batchErrors" or "dmlRowCounts" to true is only permitted for DML statements');
 messages.set(ERR_INVALID_BIND_NAME,                     // NJS-097
   'no bind placeholder named ":%s" was found in the statement text');
-messages.set(ERR_WRONG_NUMBER_OF_POSITIONAL_BINDS,      // NJS-098
-  '%s positional bind values are required but %s were provided');
+messages.set(ERR_WRONG_NUMBER_OF_BINDS,                 // NJS-098
+  '%s bind placeholders were used in the SQL statement but %s bind values were provided');
 messages.set(ERR_BUFFER_LENGTH_INSUFFICIENT,            // NJS-099
   'internal error: buffer of length %s insufficient to hold %s bytes');
 messages.set(ERR_NCHAR_CS_NOT_SUPPORTED,                // NJS-100
@@ -841,7 +841,7 @@ module.exports = {
   ERR_CONFIG_PROVIDER_LOAD_FAILED,
   ERR_WALLET_TYPE_NOT_SUPPORTED,
   ERR_INVALID_BIND_NAME,
-  ERR_WRONG_NUMBER_OF_POSITIONAL_BINDS,
+  ERR_WRONG_NUMBER_OF_BINDS,
   ERR_BUFFER_LENGTH_INSUFFICIENT,
   ERR_NCHAR_CS_NOT_SUPPORTED,
   ERR_MISSING_CREDENTIALS,

lib/thin/connection.js

@@ -170,11 +170,22 @@ class ThinConnectionImpl extends ConnectionImpl {
   async _execute(statement, numIters, binds, options, executeManyFlag) {
 
     // perform binds
-    const numBinds = statement.bindInfoList.length;
-    const numVars = binds.length;
-    if (numBinds !== numVars && (numVars === 0 || !binds[0].name)) {
-      errors.throwErr(errors.ERR_WRONG_NUMBER_OF_POSITIONAL_BINDS, numBinds, numVars);
+    const numStmtBinds = statement.bindInfoList.length;
+    const numUserBinds = binds.length;
+    if (numStmtBinds !== numUserBinds) {
+      if (!binds[0]?.name) {
+        // positional bind mismatch or zero binds
+        errors.throwErr(errors.ERR_WRONG_NUMBER_OF_BINDS, numStmtBinds, numUserBinds);
+      }
+
+      // named bind mismatch
+      // Find the number of distinct named bind placeholders and see
+      // if they match the bind values
+      const numDistinctStmtBinds = statement.bindInfoDict.size;
+      if (numDistinctStmtBinds !== numUserBinds)
+        errors.throwErr(errors.ERR_WRONG_NUMBER_OF_BINDS, numStmtBinds, numUserBinds);
     }
+
     for (let i = 0; i < binds.length; i++) {
       await this._bind(statement, binds[i], i + 1);
     }

test/binding.js

@@ -777,10 +777,61 @@ describe('4. binding.js', function() {
           await connection.execute(sql);
         },
         //ORA-01008: not all variables bound
-        //NJS-098: 1 positional bind values are required but 0 were provided
+        //NJS-098: 1 bind placeholders were used in the SQL statement but 0 bind values were provided
         /ORA-01008:|NJS-098:/
       );
     });
+
+    it('4.9.4 not using a bind name in execute statement - repeated bind placeholders', async function() {
+      const sql = 'select :val, bind(:val) from dual';
+      await assert.rejects(
+        async () => {
+          await connection.execute(sql);
+        },
+        //ORA-01008: not all variables bound
+        //NJS-098: 2 bind placeholders were used in the SQL statement but 0 bind values were provided
+        /ORA-01008:|NJS-098:/
+      );
+    });
+
+    it('4.9.5 mismatch of named binds - lower number of bind values', async function() {
+      const sql = 'select :val1, :val2 from dual';
+      const binds = {val1: 1};
+      await assert.rejects(
+        async () => {
+          await connection.execute(sql, binds);
+        },
+        //ORA-01008: not all variables bound
+        //NJS-098: 2 bind placeholders were used in the SQL statement but 1 bind values were provided
+        /ORA-01008:|NJS-098:/
+      );
+    });
+
+    it('4.9.6 mismatch of named repeated binds - lower number of bind values', async function() {
+      const sql = 'select :val1, bind(:val1), :val2 from dual';
+      const binds = {val1: 1};
+      await assert.rejects(
+        async () => {
+          await connection.execute(sql, binds);
+        },
+        //ORA-01008: not all variables bound
+        //NJS-098: 3 bind placeholders were used in the SQL statement but 1 bind values were provided
+        /ORA-01008:|NJS-098:/
+      );
+    });
+
+    it('4.9.7 mismatch of named binds - higher number of bind values', async function() {
+      const sql = 'select :val1 from dual';
+      const binds = {val1: 1, val2: 2};
+      await assert.rejects(
+        async () => {
+          await connection.execute(sql, binds);
+        },
+        //ORA-01036: illegal variable name/number
+        //NJS-098: 1 bind placeholders were used in the SQL statement but 2 bind values were provided
+        /ORA-01036:|NJS-098:/
+      );
+    });
   }); // 4.9
 
   describe('4.10 various quoted binds', function() {
@@ -1187,4 +1238,48 @@ describe('4. binding.js', function() {
         assert(newOpenCount - openCount < 4);
       });
   });
+
+  describe('4.16 positional binding', function() {
+    let connection = null;
+    before(async function() {
+      connection = await oracledb.getConnection(dbConfig);
+    });
+
+    after(async function() {
+      await connection.close();
+    });
+
+    it('4.16.1 Matching bind values', async function() {
+      const sql = 'select :1, :2, :3 from dual';
+      const binds = [1, 2, 3];
+      const result = await connection.execute(sql, binds);
+      assert.strictEqual(result.rows[0][0], 1);
+      assert.strictEqual(result.rows[0][2], 3);
+    });
+
+    it('4.16.2 Negative - bind mismatch of zero bind values', async function() {
+      const sql = 'select :1, dump(:2) from dual';
+      await assert.rejects(
+        async () => {
+          await connection.execute(sql);
+        },
+        //ORA-01008: not all variables bound
+        //NJS-098: 2 bind placeholders were used in the SQL statement but 0 bind values were provided
+        /ORA-01008:|NJS-098:/
+      );
+    });
+
+    it('4.16.3 Negative - bind mismatch of non-zero bind values', async function() {
+      const sql = 'select :1, dump(:2), :3 from dual';
+      const binds = [1];
+      await assert.rejects(
+        async () => {
+          await connection.execute(sql, binds);
+        },
+        //ORA-01008: not all variables bound
+        //NJS-098: 3 bind placeholders were used in the SQL statement but 1 bind values were provided
+        /ORA-01008:|NJS-098:/
+      );
+    });
+  });
 });

test/list.txt

@@ -212,6 +212,10 @@ Overview of node-oracledb functional tests
         4.9.1 test case sensitivity of quoted bind names
         4.9.2 using a reserved keyword as a bind name
         4.9.3 not using a bind name in execute statement
+        4.9.4 not using a bind name in execute statement - repeated bind placeholders
+        4.9.5 mismatch of named binds - lower number of bind values
+        4.9.6 mismatch of named repeated binds - lower number of bind values
+        4.9.7 mismatch of named binds - higher number of bind values
     4.10 various quoted binds
         4.10.1 various quoted bind names
     4.11 PL/SQL block with null Binds
@@ -228,6 +232,10 @@ Overview of node-oracledb functional tests
     4.15 binding different data types for same sql
         4.15.1 change bindtypes using bindByPosition for queries
         4.15.2 change bindtypes using bindByName for queries
+    4.16 positional binding
+        4.16.1 Matching bind values
+        4.16.2 Negative - bind mismatch of zero bind values
+        4.16.3 Negative - bind mismatch of non-zero bind values
 
 5. externalAuth.js
     5.1 tests that work both when DB has configured externalAuth and not configured