Skip to content

Commit 81ed913

Browse files
authored
fix: send named HTTP parameters as an object map; test parameterized queries against a live runtime in CI (#322)
test: cover parameterized queries against a live local runtime; fix named-parameter HTTP format
1 parent 7d7b99c commit 81ed913

3 files changed

Lines changed: 148 additions & 8 deletions

File tree

.github/workflows/test.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ jobs:
130130
sleep 2
131131
132132
- name: Run Node.js local runtime tests
133-
run: npm run test:node -- test/local-runtime.test.ts test/user-agent.test.ts test/grpc-http-fallback.test.ts
133+
run: npm run test:node -- test/local-runtime.test.ts test/local-runtime-params.test.ts test/user-agent.test.ts test/grpc-http-fallback.test.ts
134134

135135
- name: Run performance tests
136136
run: npm run test:perf

src/client-common.ts

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -661,7 +661,9 @@ export class SpiceClient {
661661
/**
662662
* Converts parameters for HTTP endpoint format
663663
*/
664-
private convertParametersForHttp(parameters?: QueryParameters): any[] {
664+
private convertParametersForHttp(
665+
parameters?: QueryParameters,
666+
): any[] | Record<string, any> {
665667
if (!parameters) {
666668
return [];
667669
}
@@ -684,8 +686,10 @@ export class SpiceClient {
684686
return extractedVal;
685687
});
686688
} else {
687-
// Named parameters - convert to array of {name, value} objects
688-
return Object.entries(parameters).map(([name, value]) => {
689+
// Named parameters - the runtime expects a plain JSON object map
690+
// ({"name": value}); nested objects such as [{name, value}] are rejected
691+
const converted: Record<string, any> = {};
692+
for (const [name, value] of Object.entries(parameters)) {
689693
const extractedValue = this.extractParamValue(value);
690694
let serializedValue: any = extractedValue;
691695
if (extractedValue instanceof Date)
@@ -699,9 +703,9 @@ export class SpiceClient {
699703
) {
700704
serializedValue = (extractedValue as any).toString('base64');
701705
}
702-
703-
return { name, value: serializedValue };
704-
});
706+
converted[name] = serializedValue;
707+
}
708+
return converted;
705709
}
706710
}
707711

@@ -845,9 +849,13 @@ export class SpiceClient {
845849
// runtime, and only when Content-Type is exactly application/json.
846850
// Spice Cloud parses every request body as raw SQL, so queries without
847851
// parameters are sent as plain text — the format every endpoint accepts.
852+
const hasHttpParameters = Array.isArray(httpParameters)
853+
? httpParameters.length > 0
854+
: Object.keys(httpParameters).length > 0;
855+
848856
let requestBody: string;
849857
let contentType: string;
850-
if (httpParameters.length === 0) {
858+
if (!hasHttpParameters) {
851859
requestBody = queryText;
852860
contentType = 'text/plain';
853861
} else if (this._isSpiceCloud) {

test/local-runtime-params.test.ts

Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,132 @@
1+
import { SpiceClient } from '../';
2+
3+
/**
4+
* Parameterized query tests against a live local Spice runtime.
5+
*
6+
* These run in the Local Runtime CI job, which starts a real `spiced`, so they
7+
* exercise the full parameter path end-to-end over gRPC Arrow Flight — unlike
8+
* the unit tests, which never touch a server.
9+
*
10+
* The assertions are value-based on anchored expressions, deliberately not
11+
* pinned to the parameter transport: they must hold whether parameters are
12+
* substituted client-side or bound server-side as Flight SQL prepared
13+
* statements, so the suite guards a migration between the two.
14+
*/
15+
describe('local runtime parameterized queries', () => {
16+
const client = new SpiceClient();
17+
18+
describe('positional parameters', () => {
19+
test('numeric parameters in expressions', async () => {
20+
const table = await client.sql('SELECT $1 + $2 AS total', {
21+
parameters: [40, 2],
22+
});
23+
const rows = table.toArray();
24+
expect(rows).toHaveLength(1);
25+
expect(Number(rows[0].total)).toBe(42);
26+
});
27+
28+
test('string parameter round-trips', async () => {
29+
const table = await client.sql('SELECT upper($1) AS u, $2 AS raw', {
30+
parameters: ['abc', 'hello world'],
31+
});
32+
const row = table.toArray()[0];
33+
expect(String(row.u)).toBe('ABC');
34+
expect(String(row.raw)).toBe('hello world');
35+
});
36+
37+
test('boolean parameter drives a CASE expression', async () => {
38+
const table = await client.sql(
39+
"SELECT CASE WHEN $1 THEN 'yes' ELSE 'no' END AS answer",
40+
{ parameters: [true] },
41+
);
42+
expect(String(table.toArray()[0].answer)).toBe('yes');
43+
});
44+
45+
test('parameters in a comparison predicate', async () => {
46+
const table = await client.sql('SELECT ($1 > $2) AS gt, ($1 = $1) AS eq', {
47+
parameters: [10, 3],
48+
});
49+
const row = table.toArray()[0];
50+
expect(Boolean(row.gt)).toBe(true);
51+
expect(Boolean(row.eq)).toBe(true);
52+
});
53+
54+
test('ten or more positional parameters keep their positions', async () => {
55+
// $1 vs $10 disambiguation is a classic substitution/binding bug source
56+
const table = await client.sql(
57+
'SELECT $10 AS last, $1 AS first, $2 + $3 + $4 + $5 + $6 + $7 + $8 + $9 AS mid',
58+
{ parameters: [1, 2, 3, 4, 5, 6, 7, 8, 9, 100] },
59+
);
60+
const row = table.toArray()[0];
61+
expect(Number(row.first)).toBe(1);
62+
expect(Number(row.mid)).toBe(44);
63+
expect(Number(row.last)).toBe(100);
64+
});
65+
});
66+
67+
describe('named parameters', () => {
68+
test('named parameters resolve by name, not position', async () => {
69+
const table = await client.sql('SELECT $b - $a AS diff', {
70+
parameters: { a: 2, b: 50 },
71+
});
72+
expect(Number(table.toArray()[0].diff)).toBe(48);
73+
});
74+
75+
test('a named parameter used twice binds both sites', async () => {
76+
const table = await client.sql('SELECT $n * $n AS squared', {
77+
parameters: { n: 7 },
78+
});
79+
expect(Number(table.toArray()[0].squared)).toBe(49);
80+
});
81+
82+
test('overlapping names resolve to the longest match', async () => {
83+
const table = await client.sql('SELECT $name AS a, $name_long AS b', {
84+
parameters: { name: 'short', name_long: 'long' },
85+
});
86+
const row = table.toArray()[0];
87+
expect(String(row.a)).toBe('short');
88+
expect(String(row.b)).toBe('long');
89+
});
90+
});
91+
92+
describe('values stay data', () => {
93+
test('SQL syntax inside a string parameter is not executed', async () => {
94+
const hostile = "'; SELECT 999 AS pwned; --";
95+
const table = await client.sql('SELECT $1 AS v, 1 AS marker', {
96+
parameters: [hostile],
97+
});
98+
const rows = table.toArray();
99+
expect(rows).toHaveLength(1);
100+
expect(String(rows[0].v)).toBe(hostile);
101+
expect(Number(rows[0].marker)).toBe(1);
102+
});
103+
104+
test('quotes and backslashes round-trip intact', async () => {
105+
const tricky = `it's a "test" with \\ and ''`;
106+
const table = await client.sql('SELECT $1 AS v', {
107+
parameters: [tricky],
108+
});
109+
expect(String(table.toArray()[0].v)).toBe(tricky);
110+
});
111+
});
112+
113+
describe('HTTP transport', () => {
114+
const httpClient = new SpiceClient({ httpOnly: true });
115+
116+
test('positional parameters over the HTTP endpoint', async () => {
117+
const table = await httpClient.sql('SELECT $1 + $2 AS total', {
118+
parameters: [20, 22],
119+
});
120+
const rows = table.toArray();
121+
expect(rows).toHaveLength(1);
122+
expect(Number(rows[0].total)).toBe(42);
123+
});
124+
125+
test('named parameters over the HTTP endpoint', async () => {
126+
const table = await httpClient.sql('SELECT upper($word) AS u', {
127+
parameters: { word: 'spice' },
128+
});
129+
expect(String(table.toArray()[0].u)).toBe('SPICE');
130+
});
131+
});
132+
});

0 commit comments

Comments
 (0)