Batch INSERT (using array of arrays) results in the application hanging (#261)

CHANGELOG.md

@@ -3,6 +3,7 @@
 ## Unreleased
 
 - Report untyped NULL columns as STRING in kernel result metadata, matching Thrift. (#470)
+- Reject non-scalar query parameter values at bind time with a `ParameterError` on both the Thrift and kernel backends, instead of stringifying them (which could leave a Thrift operation stuck before FINISHED). Newly rejected on the Thrift backend: arrays, plain objects, boxed primitives (e.g. `new String('x')`, `new Number(1)`), functions, symbols, and a `DBSQLParameter` that wraps another `DBSQLParameter`. Primitives, `Date`, `Int64`, and `DBSQLParameter` are still accepted.
 
 ## 3.0.0
 

lib/kernel/KernelInputValidation.ts

@@ -12,35 +12,14 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-import Int64 from 'node-int64';
-import { DBSQLParameter, DBSQLParameterValue } from '../DBSQLParameter';
-import ParameterError from '../errors/ParameterError';
+import assertBindableValue from '../utils/assertBindableValue';
 
 /**
- * Reject a parameter value that cannot be bound as a scalar. Arrays and plain
- * objects stringify to garbage (e.g. `[1,2,3]` → `"1,2,3"`) that the server
- * fails to coerce — on the Thrift path the operation never returns to
- * FINISHED (a DoS hazard), and on kernel it surfaces an opaque server error. We
- * fail fast at bind time instead, mirroring the kernel's compound-type
- * rejection. `DBSQLParameter`, `Int64`, `Date`, and JS primitives are allowed.
- *
- * Parameter-marker counting and arity validation are intentionally NOT done
- * here: the kernel's `statement::params` codec owns that check and binds
- * exactly one placeholder style per statement, so duplicating the SQL-walk
- * JS-side would only risk drift. The driver's sole bind-time job is this
- * cheap, type-shape gate before the value crosses the napi boundary.
+ * @deprecated Moved to a backend-neutral location — import the default export
+ * of `lib/utils/assertBindableValue` instead. Kept only as a thin shim because
+ * the package has no `exports` map, so `dist/kernel/KernelInputValidation` is a
+ * reachable deep-import path; it will be removed in a future release.
  */
-export default function assertBindableValue(value: DBSQLParameter | DBSQLParameterValue, label: string): void {
-  if (value instanceof DBSQLParameter) return;
-  if (value === null || value === undefined) return;
-  if (Array.isArray(value)) {
-    throw new ParameterError(
-      `${label} is an array; compound types (ARRAY/MAP/STRUCT) are not bindable as a parameter value`,
-    );
-  }
-  if (typeof value === 'object' && !(value instanceof Date) && !(value instanceof Int64)) {
-    throw new ParameterError(
-      `${label} is an object; only scalar values (string/number/bigint/boolean), Date, and Int64 are bindable`,
-    );
-  }
+export default function kernelAssertBindableValue(...args: Parameters<typeof assertBindableValue>): void {
+  assertBindableValue(...args);
 }

lib/kernel/KernelPositionalParams.ts

@@ -15,7 +15,7 @@
 import { DBSQLParameter, DBSQLParameterValue } from '../DBSQLParameter';
 import ParameterError from '../errors/ParameterError';
 import { KernelNativeRawParameterInput } from './KernelNativeLoader';
-import assertBindableValue from './KernelInputValidation';
+import assertBindableValue from '../utils/assertBindableValue';
 
 /**
  * Derive `(precision,scale)` from a decimal value string for the kernel

lib/thrift-backend/ThriftSessionBackend.ts

@@ -38,6 +38,7 @@ import escapeCatalogName from '../utils/escapeCatalogName';
 import EmptyOperationBackend from './EmptyOperationBackend';
 import isObjectNotFound from './metadataErrors';
 import createPrimaryKeysSchema from './primaryKeysSchema';
+import assertBindableValue from '../utils/assertBindableValue';
 
 interface OperationResponseShape {
   status: TStatus;
@@ -118,16 +119,18 @@ function getQueryParameters(
   if (namedParameters !== undefined) {
     for (const name of Object.keys(namedParameters)) {
       const value = namedParameters[name];
+      assertBindableValue(value, `namedParameters[${name}]`);
       const param = value instanceof DBSQLParameter ? value : new DBSQLParameter({ value });
       result.push(param.toSparkParameter({ name }));
     }
   }
 
   if (ordinalParameters !== undefined) {
-    for (const value of ordinalParameters) {
+    ordinalParameters.forEach((value, i) => {
+      assertBindableValue(value, `ordinalParameters[${i}]`);
       const param = value instanceof DBSQLParameter ? value : new DBSQLParameter({ value });
       result.push(param.toSparkParameter());
-    }
+    });
   }
 
   return result;

lib/utils/assertBindableValue.ts

@@ -0,0 +1,67 @@
+// Copyright (c) 2026 Databricks, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+import Int64 from 'node-int64';
+import { DBSQLParameter, DBSQLParameterType, DBSQLParameterValue } from '../DBSQLParameter';
+import ParameterError from '../errors/ParameterError';
+
+/**
+ * Backend-neutral bind-time gate shared by the Thrift and kernel backends.
+ *
+ * Rejects a parameter value that cannot be bound as a scalar. Arrays and plain
+ * objects stringify to garbage (e.g. `[1,2,3]` → `"1,2,3"`) that the server
+ * fails to coerce — on the Thrift path the operation never returns to
+ * FINISHED (a DoS hazard), and on the kernel path it surfaces an opaque server
+ * error. We fail fast at bind time instead, with the same error on both
+ * backends. `DBSQLParameter`, `Int64`, `Date`, and JS primitives are allowed.
+ *
+ * Parameter-marker counting and arity validation are intentionally NOT done
+ * here: that is owned by the server (Thrift) or the kernel's
+ * `statement::params` codec, so duplicating the SQL-walk JS-side would only
+ * risk drift. This is purely a cheap type-shape check.
+ */
+export default function assertBindableValue(value: DBSQLParameter | DBSQLParameterValue, label: string): void {
+  if (value instanceof DBSQLParameter) {
+    // An explicit VOID type ignores the wrapped value (binds NULL), so there is
+    // nothing to validate. Otherwise validate the unwrapped value: a
+    // non-scalar wrapped in `DBSQLParameter` hits the same STRING fallback.
+    if (value.type === DBSQLParameterType.VOID) return;
+    // A nested `DBSQLParameter` is not a valid wrapped value: the outer
+    // parameter would fall through to the STRING default with an object value.
+    if ((value.value as unknown) instanceof DBSQLParameter) {
+      throw new ParameterError(`${label} wraps a nested DBSQLParameter; the wrapped value must be a scalar`);
+    }
+    assertBindableValue(value.value, label);
+    return;
+  }
+  if (value === null || value === undefined) return;
+  if (Array.isArray(value)) {
+    throw new ParameterError(
+      `${label} is an array; compound types (ARRAY/MAP/STRUCT) are not bindable as a parameter value`,
+    );
+  }
+  if (typeof value === 'object') {
+    if (value instanceof Date || value instanceof Int64) return;
+    throw new ParameterError(
+      `${label} is an object; only scalar values (string/number/bigint/boolean), Date, and Int64 are bindable`,
+    );
+  }
+  // Allow-list the remaining primitives; rejects e.g. functions and symbols.
+  const kind = typeof value;
+  if (kind !== 'string' && kind !== 'number' && kind !== 'bigint' && kind !== 'boolean') {
+    throw new ParameterError(
+      `${label} is a ${kind}; only scalar values (string/number/bigint/boolean), Date, and Int64 are bindable`,
+    );
+  }
+}

tests/e2e/query_parameters.test.ts

@@ -231,4 +231,56 @@ describe('Query parameters', () => {
       expect(error).to.be.instanceof(ParameterError);
     }
   });
+
+  // https://github.com/databricks/databricks-sql-nodejs/issues/261
+  describe('non-scalar parameter values (issue #261)', () => {
+    // Non-scalar values are rejected client-side (before any RPC is sent), so executeStatement
+    // rejects immediately; this exercises the real session open/close round-trip around it.
+    async function executeAndCaptureError(
+      statement: string,
+      options: Parameters<Awaited<ReturnType<typeof openSession>>['executeStatement']>[1],
+    ): Promise<unknown> {
+      const session = await openSession();
+      try {
+        await session.executeStatement(statement, options);
+        return undefined;
+      } catch (error) {
+        return error;
+      } finally {
+        await session.close();
+      }
+    }
+
+    it('should reject an array-of-arrays of ordinal parameters with ParameterError instead of hanging', async () => {
+      const error = await executeAndCaptureError('SELECT ? AS c', { ordinalParameters: [[1], [2], [3]] as never });
+      expect(error).to.be.instanceof(ParameterError);
+      expect((error as Error).message).to.match(/array/);
+    });
+
+    it('should reject an array value for a named parameter with ParameterError', async () => {
+      const error = await executeAndCaptureError('SELECT :p AS c', { namedParameters: { p: [1, 2] as never } });
+      expect(error).to.be.instanceof(ParameterError);
+      expect((error as Error).message).to.match(/array/);
+    });
+
+    it('should reject a plain-object ordinal parameter value with ParameterError', async () => {
+      const error = await executeAndCaptureError('SELECT ? AS c', { ordinalParameters: [{ a: 1 } as never] });
+      expect(error).to.be.instanceof(ParameterError);
+      expect((error as Error).message).to.match(/object/);
+    });
+
+    it('should support multi-row inserts with flattened scalar ordinal parameters', async () => {
+      const session = await openSession();
+      try {
+        const operation = await session.executeStatement('SELECT * FROM VALUES (?), (?), (?) AS t(c) ORDER BY c', {
+          ordinalParameters: [1, 2, 3],
+        });
+        const result = await operation.fetchAll();
+        await operation.close();
+        expect(result).to.deep.equal([{ c: 1 }, { c: 2 }, { c: 3 }]);
+      } finally {
+        await session.close();
+      }
+    });
+  });
 });

tests/unit/DBSQLSession.test.ts

@@ -8,6 +8,7 @@ import DBSQLOperation from '../../lib/DBSQLOperation';
 import { PrimaryKeysRequest } from '../../lib/contracts/IDBSQLSession';
 import ISessionBackend from '../../lib/contracts/ISessionBackend';
 import ParameterError from '../../lib/errors/ParameterError';
+import { DBSQLParameter } from '../../lib/DBSQLParameter';
 import { TSessionHandle, TProtocolVersion } from '../../thrift/TCLIService_types';
 import ClientContextStub from './.stubs/ClientContextStub';
 import { createSessionForTest } from './.stubs/createSessionForTest';
@@ -263,6 +264,65 @@ describe('DBSQLSession', () => {
     });
   });
 
+  // https://github.com/databricks/databricks-sql-nodejs/issues/261
+  describe('executeStatement with non-scalar parameter values', () => {
+    const invalidCases: Array<{ desc: string; options: object; pattern: RegExp }> = [
+      { desc: 'ordinal array-of-arrays', options: { ordinalParameters: [[1], [2], [3]] }, pattern: /array/ },
+      { desc: 'named array value', options: { namedParameters: { p: [1, 2] } }, pattern: /array/ },
+      { desc: 'ordinal plain object', options: { ordinalParameters: [{ a: 1 }] }, pattern: /object/ },
+      { desc: 'named plain object', options: { namedParameters: { p: { a: 1 } } }, pattern: /object/ },
+      {
+        desc: 'ordinal array wrapped in DBSQLParameter',
+        options: { ordinalParameters: [new DBSQLParameter({ value: [1, 2] as never })] },
+        pattern: /array/,
+      },
+      // eslint-disable-next-line no-new-wrappers
+      { desc: 'ordinal boxed String', options: { ordinalParameters: [new String('x')] }, pattern: /object/ },
+      // eslint-disable-next-line no-new-wrappers
+      { desc: 'named boxed Number', options: { namedParameters: { p: new Number(1) } }, pattern: /object/ },
+    ];
+
+    for (const { desc, options, pattern } of invalidCases) {
+      it(`should throw ParameterError and not call the driver for ${desc}`, async () => {
+        const context = new ClientContextStub();
+        const driver = sinon.spy(context.driver);
+        const session = createSessionForTest({
+          handle: sessionHandleStub,
+          context,
+          serverProtocolVersion: TProtocolVersion.SPARK_CLI_SERVICE_PROTOCOL_V8,
+        });
+
+        try {
+          await session.executeStatement('SELECT ?', options as never);
+          expect.fail('It should throw an error');
+        } catch (error) {
+          if (error instanceof AssertionError) {
+            throw error;
+          }
+          expect(error).instanceOf(ParameterError);
+          expect((error as Error).message).to.match(pattern);
+        }
+        expect(driver.executeStatement.called).to.be.false;
+      });
+    }
+
+    it('should keep accepting valid scalar values', async () => {
+      const context = new ClientContextStub();
+      const driver = sinon.spy(context.driver);
+      const session = createSessionForTest({
+        handle: sessionHandleStub,
+        context,
+        serverProtocolVersion: TProtocolVersion.SPARK_CLI_SERVICE_PROTOCOL_V8,
+      });
+
+      await session.executeStatement('SELECT ?, ?, ?, ?, ?', {
+        ordinalParameters: [new Date('2023-09-06T03:14:27.843Z'), new Int64(1234), BigInt(1234), null, 'x'],
+      });
+      expect(driver.executeStatement.callCount).to.eq(1);
+      expect(driver.executeStatement.firstCall.args[0].parameters?.length).to.eq(5);
+    });
+  });
+
   describe('executeStatement with queryTags', () => {
     it('should set confOverlay with query_tags when queryTags are provided', async () => {
       const context = new ClientContextStub();

tests/unit/kernel/inputValidation.test.ts

@@ -14,11 +14,22 @@
 
 import { expect } from 'chai';
 import Int64 from 'node-int64';
-import assertBindableValue from '../../../lib/kernel/KernelInputValidation';
+import assertBindableValue from '../../../lib/utils/assertBindableValue';
+import kernelAssertBindableValue from '../../../lib/kernel/KernelInputValidation';
 import { DBSQLParameter, DBSQLParameterType } from '../../../lib/DBSQLParameter';
 import ParameterError from '../../../lib/errors/ParameterError';
 
-describe('KernelInputValidation.assertBindableValue', () => {
+describe('KernelInputValidation (deprecated shim)', () => {
+  it('still delegates to assertBindableValue', () => {
+    expect(() => kernelAssertBindableValue(42, 'p')).to.not.throw();
+    expect(() => kernelAssertBindableValue([1, 2, 3] as never, 'ordinalParameters[0]')).to.throw(
+      ParameterError,
+      /ordinalParameters\[0\] is an array/,
+    );
+  });
+});
+
+describe('utils/assertBindableValue', () => {
   it('accepts scalars, Date, Int64, bigint, null, and DBSQLParameter', () => {
     expect(() => assertBindableValue(42, 'p')).to.not.throw();
     expect(() => assertBindableValue('x', 'p')).to.not.throw();
@@ -39,4 +50,44 @@ describe('KernelInputValidation.assertBindableValue', () => {
   it('rejects plain objects', () => {
     expect(() => assertBindableValue({ a: 1 } as never, 'p')).to.throw(ParameterError, /object/);
   });
+
+  it('rejects boxed primitives', () => {
+    // eslint-disable-next-line no-new-wrappers
+    expect(() => assertBindableValue(new String('x') as never, 'p')).to.throw(ParameterError, /p is an object/);
+    // eslint-disable-next-line no-new-wrappers
+    expect(() => assertBindableValue(new Number(1) as never, 'p')).to.throw(ParameterError, /p is an object/);
+    // eslint-disable-next-line no-new-wrappers
+    expect(() => assertBindableValue(new Boolean(true) as never, 'p')).to.throw(ParameterError, /p is an object/);
+  });
+
+  it('rejects non-scalar values wrapped in DBSQLParameter', () => {
+    expect(() => assertBindableValue(new DBSQLParameter({ value: [1, 2] as never }), 'ordinalParameters[0]')).to.throw(
+      ParameterError,
+      /ordinalParameters\[0\] is an array/,
+    );
+    expect(() =>
+      assertBindableValue(new DBSQLParameter({ type: DBSQLParameterType.STRING, value: { a: 1 } as never }), 'p'),
+    ).to.throw(ParameterError, /object/);
+  });
+
+  it('rejects a DBSQLParameter nested inside a DBSQLParameter', () => {
+    expect(() =>
+      assertBindableValue(new DBSQLParameter({ value: new DBSQLParameter({ value: 1 }) as never }), 'p'),
+    ).to.throw(ParameterError, /nested DBSQLParameter/);
+  });
+
+  it('rejects functions and symbols', () => {
+    expect(() => assertBindableValue((() => 1) as never, 'p')).to.throw(ParameterError, /p is a function/);
+    expect(() => assertBindableValue(Symbol('s') as never, 'p')).to.throw(ParameterError, /p is a symbol/);
+    expect(() => assertBindableValue(new DBSQLParameter({ value: (() => 1) as never }), 'p')).to.throw(
+      ParameterError,
+      /function/,
+    );
+  });
+
+  it('accepts DBSQLParameter with explicit VOID type regardless of value', () => {
+    expect(() =>
+      assertBindableValue(new DBSQLParameter({ type: DBSQLParameterType.VOID, value: [1, 2] as never }), 'p'),
+    ).to.not.throw();
+  });
 });

tests/unit/kernel/positionalParams.test.ts

@@ -102,6 +102,22 @@ describe('KernelPositionalParams.buildKernelPositionalParams', () => {
       { sqlType: 'TIMESTAMP', value: '2024-01-15 10:30:00' },
     ]);
   });
+
+  it('rejects a non-scalar wrapped in DBSQLParameter with the same ParameterError as Thrift', () => {
+    expect(() =>
+      buildKernelPositionalParams([new DBSQLParameter({ type: DBSQLParameterType.STRING, value: { a: 1 } as never })]),
+    ).to.throw(ParameterError, /ordinalParameters\[0\] is an object/);
+    expect(() => buildKernelPositionalParams([new DBSQLParameter({ value: [1, 2] as never })])).to.throw(
+      ParameterError,
+      /ordinalParameters\[0\] is an array/,
+    );
+  });
+
+  it('allows an explicit VOID DBSQLParameter regardless of its wrapped value', () => {
+    expect(
+      buildKernelPositionalParams([new DBSQLParameter({ type: DBSQLParameterType.VOID, value: [1, 2] as never })]),
+    ).to.deep.equal([{ sqlType: 'VOID' }]);
+  });
 });
 
 describe('KernelPositionalParams.buildKernelNamedParams', () => {
@@ -135,4 +151,12 @@ describe('KernelPositionalParams.buildKernelNamedParams', () => {
       }),
     ).to.deep.equal([{ name: 'duration', sqlType: 'INTERVAL MONTH', value: '2-6' }]);
   });
+
+  it('rejects a named non-scalar wrapped in DBSQLParameter with ParameterError', () => {
+    expect(() =>
+      buildKernelNamedParams({
+        p: new DBSQLParameter({ type: DBSQLParameterType.STRING, value: { a: 1 } as never }),
+      }),
+    ).to.throw(ParameterError, /namedParameters\[p\] is an object/);
+  });
 });