Skip to content

Commit 7c0fada

Browse files
fix(shared): sql.number infers INT (not BIGINT) for LIMIT/OFFSET compatibility
Empirical testing against e2-dogfood.staging (from @calvarjorge) showed that Spark's LIMIT/OFFSET operators require IntegerType specifically — LongType/BIGINT is rejected with INVALID_LIMIT_LIKE_EXPRESSION.DATA_TYPE: The offset expression must be integer type, but got "BIGINT". So the original PR's "LIMIT now Just Works" claim was false: BIGINT fails for the exact motivating case. Catalyst auto-widens INT to BIGINT/DECIMAL/DOUBLE for wider columns, so defaulting to INT is strictly better than defaulting to BIGINT. Change sql.number inference: - JS integer in [-2^31, 2^31 - 1] → INT (was BIGINT) - JS integer outside INT but within MAX_SAFE_INTEGER → BIGINT - integer-shaped string in INT range → INT (was BIGINT) - integer-shaped string outside INT, within BIGINT → BIGINT - everything else unchanged (DOUBLE for non-integer, NUMERIC for decimal strings, throw for out-of-BIGINT and non-numeric) Update analytics.md to recommend `-- @param limit INT` and explain the Spark IntegerType requirement. Update unit + integration tests to pin INT bindings for in-range values, with explicit boundary coverage at +/- 2^31. Regenerate the API reference page. Co-authored-by: Isaac Signed-off-by: James Broadhead <jamesbroadhead@gmail.com>
1 parent 3893551 commit 7c0fada

5 files changed

Lines changed: 124 additions & 52 deletions

File tree

docs/docs/api/appkit/Variable.sql.md

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -294,18 +294,25 @@ number(value: string | number): SQLNumberMarker;
294294

295295
Creates a numeric type parameter. The wire SQL type is inferred from the
296296
value so the parameter binds correctly in any context, including `LIMIT`
297-
and `OFFSET` (which require integer types):
297+
and `OFFSET`:
298298

299-
- JS integer (`10`) → `BIGINT`
299+
- JS integer in `[-2^31, 2^31 - 1]``INT`
300+
- JS integer outside `INT` but within `Number.MAX_SAFE_INTEGER``BIGINT`
300301
- JS non-integer (`3.14`) → `DOUBLE`
301-
- integer-shaped string (`"10"`) `BIGINT` (common HTTP-input case;
302-
works with `LIMIT :n` / `OFFSET :m`)
302+
- integer-shaped string in `INT` range `INT` (common HTTP-input case)
303+
- integer-shaped string outside `INT` but within `BIGINT``BIGINT`
303304
- decimal-shaped string (`"123.45"`) → `NUMERIC` (preserves precision)
304305

306+
Why default to `INT`? Spark's `LIMIT` and `OFFSET` operators require
307+
`IntegerType` specifically — `BIGINT` (`LongType`) is rejected with
308+
`INVALID_LIMIT_LIKE_EXPRESSION.DATA_TYPE`. Catalyst auto-widens `INT`
309+
to `BIGINT` / `DECIMAL` / `DOUBLE` for wider columns, so `INT` is a
310+
strictly better default than `BIGINT`.
311+
305312
Throws on `NaN`, `Infinity`, JS integers outside `Number.MAX_SAFE_INTEGER`,
306-
or non-numeric strings. Reach for `sql.int()`, `sql.bigint()`,
307-
`sql.float()`, `sql.double()`, or `sql.numeric()` if you need to override
308-
the inferred type.
313+
integer-shaped strings outside the `BIGINT` range, or non-numeric strings.
314+
Reach for `sql.int()`, `sql.bigint()`, `sql.float()`, `sql.double()`, or
315+
`sql.numeric()` to override the inferred type.
309316

310317
#### Parameters
311318

@@ -322,10 +329,11 @@ Marker for a numeric SQL parameter
322329
#### Example
323330

324331
```typescript
325-
sql.number(123); // { __sql_type: "BIGINT", value: "123" }
326-
sql.number(0.5); // { __sql_type: "DOUBLE", value: "0.5" }
327-
sql.number("10"); // { __sql_type: "BIGINT", value: "10" }
328-
sql.number("123.45"); // { __sql_type: "NUMERIC", value: "123.45" }
332+
sql.number(123); // { __sql_type: "INT", value: "123" }
333+
sql.number(3_000_000_000); // { __sql_type: "BIGINT", value: "3000000000" }
334+
sql.number(0.5); // { __sql_type: "DOUBLE", value: "0.5" }
335+
sql.number("10"); // { __sql_type: "INT", value: "10" }
336+
sql.number("123.45"); // { __sql_type: "NUMERIC", value: "123.45" }
329337
```
330338

331339
### numeric()

docs/docs/plugins/analytics.md

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,15 +43,17 @@ Use `:paramName` placeholders and optionally annotate parameter types using SQL
4343
```sql
4444
-- @param startDate DATE
4545
-- @param endDate DATE
46-
-- @param limit BIGINT
46+
-- @param limit INT
4747
SELECT ...
4848
WHERE usage_date BETWEEN :startDate AND :endDate
4949
LIMIT :limit
5050
```
5151

52-
`LIMIT` / `OFFSET` require an integer-typed binding (`INT` or `BIGINT`).
53-
Annotate accordingly, or use `sql.number()` (auto-infers `BIGINT` for integer
54-
inputs) / `sql.bigint()` / `sql.int()` at the call site.
52+
`LIMIT` / `OFFSET` require Spark `IntegerType` specifically — `BIGINT`
53+
(`LongType`) is rejected with `INVALID_LIMIT_LIKE_EXPRESSION.DATA_TYPE`.
54+
Annotate with `INT`, or use `sql.number()` (auto-infers `INT` for values in
55+
`[-2^31, 2^31-1]`, falling back to `BIGINT` for wider values) / `sql.int()`
56+
at the call site.
5557

5658
**Supported `-- @param` types** (case-insensitive):
5759
- `STRING`, `BOOLEAN`, `DATE`, `TIMESTAMP`, `BINARY`

packages/appkit/src/plugins/analytics/tests/query.test.ts

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ describe("QueryProcessor", () => {
3232
expect(result.statement).toBe(query);
3333
expect(result.parameters).toHaveLength(2);
3434
expect(result.parameters).toEqual([
35-
{ name: "user_id", value: "123", type: "BIGINT" },
35+
{ name: "user_id", value: "123", type: "INT" },
3636
{ name: "name", value: "Alice", type: "STRING" },
3737
]);
3838
});
@@ -167,11 +167,12 @@ describe("QueryProcessor", () => {
167167

168168
test("should not override workspace_id if already provided", async () => {
169169
const query = "SELECT * FROM data WHERE workspace_id = :workspaceId";
170+
// 9876543210 exceeds INT_MAX (2^31 - 1) so inference falls through to
171+
// BIGINT — appropriate for ID columns.
170172
const parameters = { workspaceId: sql.number("9876543210") };
171173

172174
const result = await processor.processQueryParams(query, parameters);
173175

174-
// Integer-shaped strings infer BIGINT (matches LIMIT/OFFSET pattern)
175176
expect(result.workspaceId).toEqual({
176177
__sql_type: "BIGINT",
177178
value: "9876543210",
@@ -180,7 +181,11 @@ describe("QueryProcessor", () => {
180181
});
181182

182183
describe("LIMIT / OFFSET bindings (regression for #323)", () => {
183-
test("sql.number(integer) binds as BIGINT for LIMIT/OFFSET", () => {
184+
// Spark requires IntegerType for LIMIT/OFFSET; BIGINT/LongType is
185+
// rejected with INVALID_LIMIT_LIKE_EXPRESSION.DATA_TYPE. These tests
186+
// pin INT inference so sql.number(req.query.n) works against the
187+
// warehouse without explicit casting.
188+
test("sql.number(integer) binds as INT for LIMIT/OFFSET", () => {
184189
const query = "SELECT * FROM events LIMIT :n OFFSET :m";
185190
const parameters = {
186191
n: sql.number(10),
@@ -190,12 +195,12 @@ describe("QueryProcessor", () => {
190195
const result = processor.convertToSQLParameters(query, parameters);
191196

192197
expect(result.parameters).toEqual([
193-
{ name: "n", value: "10", type: "BIGINT" },
194-
{ name: "m", value: "20", type: "BIGINT" },
198+
{ name: "n", value: "10", type: "INT" },
199+
{ name: "m", value: "20", type: "INT" },
195200
]);
196201
});
197202

198-
test("sql.number(integer-shaped string) binds as BIGINT for LIMIT/OFFSET", () => {
203+
test("sql.number(integer-shaped string) binds as INT for LIMIT/OFFSET", () => {
199204
// Express/URLSearchParams return strings — this is the common
200205
// handler pattern: sql.number(req.query.n).
201206
const query = "SELECT * FROM events LIMIT :n OFFSET :m";
@@ -207,19 +212,19 @@ describe("QueryProcessor", () => {
207212
const result = processor.convertToSQLParameters(query, parameters);
208213

209214
expect(result.parameters).toEqual([
210-
{ name: "n", value: "10", type: "BIGINT" },
211-
{ name: "m", value: "20", type: "BIGINT" },
215+
{ name: "n", value: "10", type: "INT" },
216+
{ name: "m", value: "20", type: "INT" },
212217
]);
213218
});
214219

215-
test("sql.bigint(string) binds as BIGINT for LIMIT/OFFSET", () => {
220+
test("sql.int(string) binds as INT for LIMIT/OFFSET (explicit form)", () => {
216221
const query = "SELECT * FROM events LIMIT :n";
217-
const parameters = { n: sql.bigint("10") };
222+
const parameters = { n: sql.int("10") };
218223

219224
const result = processor.convertToSQLParameters(query, parameters);
220225

221226
expect(result.parameters).toEqual([
222-
{ name: "n", value: "10", type: "BIGINT" },
227+
{ name: "n", value: "10", type: "INT" },
223228
]);
224229
});
225230
});
@@ -275,7 +280,7 @@ describe("QueryProcessor", () => {
275280
expect(result.parameters[0]).toEqual({
276281
name: "age",
277282
value: "25",
278-
type: "BIGINT",
283+
type: "INT",
279284
});
280285
});
281286

packages/shared/src/sql/helpers.ts

Lines changed: 35 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -199,35 +199,49 @@ export const sql = {
199199
/**
200200
* Creates a numeric type parameter. The wire SQL type is inferred from the
201201
* value so the parameter binds correctly in any context, including `LIMIT`
202-
* and `OFFSET` (which require integer types):
202+
* and `OFFSET`:
203203
*
204-
* - JS integer (`10`) → `BIGINT`
204+
* - JS integer in `[-2^31, 2^31 - 1]` → `INT`
205+
* - JS integer outside `INT` but within `Number.MAX_SAFE_INTEGER` → `BIGINT`
205206
* - JS non-integer (`3.14`) → `DOUBLE`
206-
* - integer-shaped string (`"10"`) → `BIGINT` (common HTTP-input case;
207-
* works with `LIMIT :n` / `OFFSET :m`)
207+
* - integer-shaped string in `INT` range → `INT` (common HTTP-input case)
208+
* - integer-shaped string outside `INT` but within `BIGINT` → `BIGINT`
208209
* - decimal-shaped string (`"123.45"`) → `NUMERIC` (preserves precision)
209210
*
211+
* Why default to `INT`? Spark's `LIMIT` and `OFFSET` operators require
212+
* `IntegerType` specifically — `BIGINT` (`LongType`) is rejected with
213+
* `INVALID_LIMIT_LIKE_EXPRESSION.DATA_TYPE`. Catalyst auto-widens `INT`
214+
* to `BIGINT` / `DECIMAL` / `DOUBLE` for wider columns, so `INT` is a
215+
* strictly better default than `BIGINT`.
216+
*
210217
* Throws on `NaN`, `Infinity`, JS integers outside `Number.MAX_SAFE_INTEGER`,
211-
* or non-numeric strings. Reach for `sql.int()`, `sql.bigint()`,
212-
* `sql.float()`, `sql.double()`, or `sql.numeric()` if you need to override
213-
* the inferred type.
218+
* integer-shaped strings outside the `BIGINT` range, or non-numeric strings.
219+
* Reach for `sql.int()`, `sql.bigint()`, `sql.float()`, `sql.double()`, or
220+
* `sql.numeric()` to override the inferred type.
214221
*
215222
* @param value - Number or numeric string
216223
* @returns Marker for a numeric SQL parameter
217224
* @example
218225
* ```typescript
219-
* sql.number(123); // { __sql_type: "BIGINT", value: "123" }
220-
* sql.number(0.5); // { __sql_type: "DOUBLE", value: "0.5" }
221-
* sql.number("10"); // { __sql_type: "BIGINT", value: "10" }
222-
* sql.number("123.45"); // { __sql_type: "NUMERIC", value: "123.45" }
226+
* sql.number(123); // { __sql_type: "INT", value: "123" }
227+
* sql.number(3_000_000_000); // { __sql_type: "BIGINT", value: "3000000000" }
228+
* sql.number(0.5); // { __sql_type: "DOUBLE", value: "0.5" }
229+
* sql.number("10"); // { __sql_type: "INT", value: "10" }
230+
* sql.number("123.45"); // { __sql_type: "NUMERIC", value: "123.45" }
223231
* ```
224232
*/
225233
number(value: number | string): SQLNumberMarker {
226234
if (typeof value === "number") {
227235
ensureFiniteNumber(value, "sql.number");
228236
if (Number.isInteger(value)) {
229237
ensureSafeInteger(value, "sql.number");
230-
return { __sql_type: "BIGINT", value: BigInt(value).toString() };
238+
const asBigInt = BigInt(value);
239+
// INT (32-bit) is required by Spark for LIMIT/OFFSET; Catalyst
240+
// widens INT → BIGINT/DECIMAL/DOUBLE automatically.
241+
if (asBigInt >= INT_MIN && asBigInt <= INT_MAX) {
242+
return { __sql_type: "INT", value: asBigInt.toString() };
243+
}
244+
return { __sql_type: "BIGINT", value: asBigInt.toString() };
231245
}
232246
return { __sql_type: "DOUBLE", value: value.toString() };
233247
}
@@ -237,20 +251,23 @@ export const sql = {
237251
`sql.number() expects number or numeric string, got: ${value === "" ? "empty string" : value}`,
238252
);
239253
}
240-
// Integer-shaped strings: emit BIGINT so HTTP-input callers
241-
// (`req.query.n` is always a string) work with LIMIT/OFFSET without
242-
// having to reach for `sql.bigint("10")` explicitly. Out-of-range
243-
// values throw — sql.numeric() is the right helper for
244-
// arbitrary-precision integers.
254+
// Integer-shaped strings get the same INT-preferring inference, so
255+
// `sql.number(req.query.n)` (Express/URLSearchParams strings) works
256+
// with LIMIT/OFFSET out of the box. Out-of-BIGINT-range throws —
257+
// sql.numeric() is the right helper for arbitrary-precision integers.
245258
if (INTEGER_LITERAL_RE.test(value)) {
259+
const parsed = BigInt(value);
246260
ensureInBigIntRange(
247-
BigInt(value),
261+
parsed,
248262
BIGINT_MIN,
249263
BIGINT_MAX,
250264
"BIGINT (64-bit signed)",
251265
"sql.number",
252266
"Use sql.numeric() with a string for arbitrary-precision integers.",
253267
);
268+
if (parsed >= INT_MIN && parsed <= INT_MAX) {
269+
return { __sql_type: "INT", value };
270+
}
254271
return { __sql_type: "BIGINT", value };
255272
}
256273
// Non-integer strings stay NUMERIC: the caller chose to pass a string,

packages/shared/src/sql/tests/sql-helpers.test.ts

Lines changed: 47 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -37,14 +37,45 @@ describe("SQL Helpers", () => {
3737
});
3838

3939
describe("number()", () => {
40-
it("should bind a JS integer as BIGINT (works in LIMIT/OFFSET)", () => {
40+
it("should bind a JS integer in INT range as INT (works with Spark LIMIT/OFFSET)", () => {
41+
// Spark requires IntegerType for LIMIT/OFFSET; BIGINT/LongType is
42+
// rejected with INVALID_LIMIT_LIKE_EXPRESSION.DATA_TYPE. INT is
43+
// auto-widened to BIGINT/DECIMAL/DOUBLE by Catalyst for wider columns.
4144
const result = sql.number(1234567890);
4245
expect(result).toEqual({
43-
__sql_type: "BIGINT",
46+
__sql_type: "INT",
4447
value: "1234567890",
4548
});
4649
});
4750

51+
it("should bind a JS integer outside INT range as BIGINT", () => {
52+
const result = sql.number(3_000_000_000);
53+
expect(result).toEqual({
54+
__sql_type: "BIGINT",
55+
value: "3000000000",
56+
});
57+
});
58+
59+
it("should bind INT boundaries correctly", () => {
60+
expect(sql.number(2147483647)).toEqual({
61+
__sql_type: "INT",
62+
value: "2147483647",
63+
});
64+
expect(sql.number(-2147483648)).toEqual({
65+
__sql_type: "INT",
66+
value: "-2147483648",
67+
});
68+
// Just past INT_MAX → BIGINT
69+
expect(sql.number(2147483648)).toEqual({
70+
__sql_type: "BIGINT",
71+
value: "2147483648",
72+
});
73+
expect(sql.number(-2147483649)).toEqual({
74+
__sql_type: "BIGINT",
75+
value: "-2147483649",
76+
});
77+
});
78+
4879
it("should bind a JS non-integer as DOUBLE", () => {
4980
const result = sql.number(3.14);
5081
expect(result).toEqual({
@@ -53,16 +84,24 @@ describe("SQL Helpers", () => {
5384
});
5485
});
5586

56-
it("should bind an integer-shaped string as BIGINT (HTTP-input case)", () => {
87+
it("should bind an integer-shaped string in INT range as INT (HTTP-input case)", () => {
5788
// Express/URLSearchParams return strings; common pattern is
58-
// sql.number(req.query.n) which must work with LIMIT/OFFSET.
89+
// sql.number(req.query.n) which must work with Spark LIMIT/OFFSET.
5990
const result = sql.number("1234567890");
6091
expect(result).toEqual({
61-
__sql_type: "BIGINT",
92+
__sql_type: "INT",
6293
value: "1234567890",
6394
});
6495
});
6596

97+
it("should bind an integer-shaped string outside INT range as BIGINT", () => {
98+
const result = sql.number("3000000000");
99+
expect(result).toEqual({
100+
__sql_type: "BIGINT",
101+
value: "3000000000",
102+
});
103+
});
104+
66105
it("should accept BIGINT-boundary integer strings", () => {
67106
expect(sql.number("9223372036854775807")).toEqual({
68107
__sql_type: "BIGINT",
@@ -112,10 +151,11 @@ describe("SQL Helpers", () => {
112151
expect(() => sql.number(Number.NaN)).toThrow(/finite number/);
113152
});
114153

115-
it("should emit canonical decimal text (no exponent) for safe integers", () => {
154+
it("should emit canonical decimal text (no exponent) for large safe integers", () => {
116155
// Sanity check: even though Number.prototype.toString could emit
117156
// exponent form for very large integers, the helper always emits
118-
// decimal text via BigInt(value).toString().
157+
// decimal text via BigInt(value).toString(). 1e15 is outside INT
158+
// range, so the wire type is BIGINT.
119159
const result = sql.number(1e15);
120160
expect(result).toEqual({
121161
__sql_type: "BIGINT",

0 commit comments

Comments
 (0)