Skip to content

Commit d210a0f

Browse files
ai: apply changes for #568 (2 review threads)
Addresses: - #4498750916 at tests/e2e/query_parameters.test.ts:242 - #4498750926 at lib/utils/assertBindableValue.ts:55 Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com>
1 parent 4af694e commit d210a0f

2 files changed

Lines changed: 29 additions & 39 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
# Release History
22

3+
## Unreleased
4+
5+
- 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.
6+
37
## 3.0.0
48

59
- Upgrade the native kernel backend to 1.4.0.

‎tests/e2e/query_parameters.test.ts‎

Lines changed: 25 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -234,67 +234,53 @@ describe('Query parameters', () => {
234234

235235
// https://github.com/databricks/databricks-sql-nodejs/issues/261
236236
describe('non-scalar parameter values (issue #261)', () => {
237-
const HANG_TIMEOUT_MS = 30000;
238-
239-
// Run the statement end-to-end against the live warehouse and return whatever error it
240-
// produced. A hang (the reported bug) surfaces as a sentinel error after HANG_TIMEOUT_MS
241-
// instead of stalling the suite.
242-
async function runAndCaptureError(
237+
// Non-scalar values are rejected client-side (before any RPC is sent), so executeStatement
238+
// rejects immediately; this exercises the real session open/close round-trip around it.
239+
async function executeAndCaptureError(
243240
statement: string,
244241
options: Parameters<Awaited<ReturnType<typeof openSession>>['executeStatement']>[1],
245242
): Promise<unknown> {
246243
const session = await openSession();
247-
let timer: NodeJS.Timeout | undefined;
248244
try {
249-
const run = (async () => {
250-
const operation = await session.executeStatement(statement, options);
251-
try {
252-
await operation.fetchAll();
253-
} finally {
254-
await operation.close().catch(() => {});
255-
}
256-
})();
257-
const hang = new Promise<never>((_, reject) => {
258-
timer = setTimeout(
259-
() => reject(new Error(`executeStatement hung for more than ${HANG_TIMEOUT_MS}ms`)),
260-
HANG_TIMEOUT_MS,
261-
);
262-
});
263-
await Promise.race([run, hang]);
245+
await session.executeStatement(statement, options);
264246
return undefined;
265247
} catch (error) {
266248
return error;
267249
} finally {
268-
if (timer) clearTimeout(timer);
269-
// Bound cleanup too: when the bug reproduces, closing the session can hang as well.
270-
await Promise.race([
271-
session.close().catch(() => {}),
272-
new Promise<void>((resolve) => {
273-
setTimeout(resolve, 5000).unref();
274-
}),
275-
]);
250+
await session.close();
276251
}
277252
}
278253

279-
it('should reject an array-of-arrays of ordinal parameters with ParameterError instead of hanging', async function () {
280-
this.timeout(HANG_TIMEOUT_MS + 30000);
281-
const error = await runAndCaptureError('SELECT ? AS c', { ordinalParameters: [[1], [2], [3]] as never });
254+
it('should reject an array-of-arrays of ordinal parameters with ParameterError instead of hanging', async () => {
255+
const error = await executeAndCaptureError('SELECT ? AS c', { ordinalParameters: [[1], [2], [3]] as never });
282256
expect(error).to.be.instanceof(ParameterError);
283257
expect((error as Error).message).to.match(/array/);
284258
});
285259

286-
it('should reject an array value for a named parameter with ParameterError', async function () {
287-
this.timeout(HANG_TIMEOUT_MS + 30000);
288-
const error = await runAndCaptureError('SELECT :p AS c', { namedParameters: { p: [1, 2] as never } });
260+
it('should reject an array value for a named parameter with ParameterError', async () => {
261+
const error = await executeAndCaptureError('SELECT :p AS c', { namedParameters: { p: [1, 2] as never } });
289262
expect(error).to.be.instanceof(ParameterError);
290263
expect((error as Error).message).to.match(/array/);
291264
});
292265

293-
it('should reject a plain-object ordinal parameter value with ParameterError', async function () {
294-
this.timeout(HANG_TIMEOUT_MS + 30000);
295-
const error = await runAndCaptureError('SELECT ? AS c', { ordinalParameters: [{ a: 1 } as never] });
266+
it('should reject a plain-object ordinal parameter value with ParameterError', async () => {
267+
const error = await executeAndCaptureError('SELECT ? AS c', { ordinalParameters: [{ a: 1 } as never] });
296268
expect(error).to.be.instanceof(ParameterError);
297269
expect((error as Error).message).to.match(/object/);
298270
});
271+
272+
it('should support multi-row inserts with flattened scalar ordinal parameters', async () => {
273+
const session = await openSession();
274+
try {
275+
const operation = await session.executeStatement('SELECT * FROM VALUES (?), (?), (?) AS t(c) ORDER BY c', {
276+
ordinalParameters: [1, 2, 3],
277+
});
278+
const result = await operation.fetchAll();
279+
await operation.close();
280+
expect(result).to.deep.equal([{ c: 1 }, { c: 2 }, { c: 3 }]);
281+
} finally {
282+
await session.close();
283+
}
284+
});
299285
});
300286
});

0 commit comments

Comments
 (0)