feat: accept BigInt values in intColumn()

[PedroHenrique0713]

Closes #58.

Picking this up since it has been open since February and @saibotsivad, who offered to send a PR, has not had the chance. Happy to step aside if they would rather land it themselves.

Implements exactly the signature @glasstiger asked for in the issue thread:

intColumn(name: string, value: number | bigint): Sender

number is kept for backwards compatibility.

Why

QuestDB's LONG is a 64-bit signed integer. JavaScript's number is a double, so anything above Number.MAX_SAFE_INTEGER (9007199254740991) is already rounded by the time it reaches intColumn() — the loss happens at the call site and nothing downstream can detect it. There was no way to write a full-range LONG from this client.

What changed

• src/buffer/base.ts — intColumn() takes number | bigint, validated the same way timestampColumn() already validates its own number | bigint parameter: typeof value !== "bigint" && !Number.isInteger(value).
• src/buffer/index.ts and src/sender.ts — signatures and JSDoc.

The wire format is unchanged: value.toString() renders a BigInt without the trailing n, so the ILP line is identical for a value expressible both ways.

Tests

Two new cases in test/sender.buffer.test.ts:

• BigInt values, positive and negative, produce the expected ILP line
• 9223372036854775807n (2^63-1, the largest LONG) survives the round trip. As a number it rounds to 9223372036854775808, which is out of range for the column

One existing test is updated: the non-integer error message now mentions BigInt, matching the wording timestampColumn() uses.

vitest passes locally, eslint and prettier --check are clean. tsc --noEmit reports one pre-existing error about conflicting undici FormData types, unchanged by this branch (same count on a clean checkout).

[glasstiger]

Review — level 3

Reviewed 7e52548 → a9aecfb. Verified against isolated worktrees of base, head, and a revert tree (head tests + base production files). Submodules: none.

Verdict: approve with comments. The change is correct, minimal, well-tested at runtime, and matches the signature agreed in #58. Three non-blocking items below, all cheap.

---

Moderate

M1 — Stale TSDoc contradicts the widened signature it sits on

src/buffer/base.ts:245-256 at head:

250:   * @param {number} value - Column value, accepts only number values.
252:   * @throws Error if the value is not an integer
254:  intColumn(name: string, value: number | bigint): SenderBuffer {
256:      throw new Error(`Value must be an integer or BigInt, received ${value}`);

The PR updated the two sibling doc blocks (src/buffer/index.ts:140,143 and src/sender.ts:302,307) but left this one behind, so the repo now documents the same method three different ways.

The natural containment argument — "SenderBufferBase is internal, nobody sees it" — does not hold. src/index.ts:12-13 exports SenderBufferV1 and SenderBufferV2, neither overrides intColumn, and typedoc.json sets visibilityFilters.inherited: true with excludeProtected: false. Regenerating typedoc at head renders intColumn(name, value: number | bigint) directly above "Column value, accepts only number values" on both class pages, while Sender.html and SenderBuffer.html from the same build say the opposite. At base the comment was accurate, so the contradiction is introduced here.

Fix: copy the updated @param / @throws lines from src/buffer/index.ts into the base.ts block. (SenderBufferV3 is not exported, so it is not part of this symptom.)

M2 — New bigint path has no INT64 range check, so out-of-range LONGs reach the wire

src/buffer/base.ts:255 checks the type, never the magnitude:

sender.table("t").intColumn("a", 2n ** 63n).at(...)

head  wire: t a=9223372036854775808i 1658484769000000000
base  throws: Value must be an integer, received 9223372036854775808

Captured from a real node:http listener over the HTTP transport, and reproduced identically on protocol v1/v2/v3 (intColumn has no subclass override). The revert tree restores the base throw, so the delta is caused by this diff.

Counterevidence, stated plainly because it is strong: the missing range check is pre-existing, and base is actually worse on the number path — intColumn("a", 2 ** 63) emits 9223372036854776000i and intColumn("a", 1e21) emits the syntactically invalid 1e+21i, at both base and head. timestampColumn and at() likewise accept unvalidated bigint. This adds a second door to an existing room rather than opening the room, which is why it is not blocking.

It is still worth handling here because this PR is specifically about writing a correct full-range LONG — the description and the new test comment both name 2^63-1 as the bound — and decimalColumn (src/buffer/bufferv3.ts:87,100) already establishes the bounds-checking convention for bigint-typed inputs in this codebase.

Fix: one comparison in the existing guard, e.g. reject when
typeof value === "bigint" && (value > 9223372036854775807n || value < -9223372036854775808n).

M3 — The headline type change is gated by nothing

tsconfig.json has include: ["src"] and there is no vitest config, so test/ is type-checked by nothing. Executed proof: changing src/sender.ts:309 back to value: number on top of head leaves tsc --noEmit (exit 0), eslint src/** (exit 0) and the full vitest suite (7 files / 174 tests) all green, and the emitted JS is byte-identical. Forcing test/ into the include surfaces the three real errors that should have been failing:

test/sender.buffer.test.ts(1002,27): error TS2345: Argument of type 'bigint' is not assignable to parameter of type 'number'.
test/sender.buffer.test.ts(1003,30): error TS2345: ...
test/sender.buffer.test.ts(1022,29): error TS2345: ...

The runtime half of the change is properly gated — the revert tree fails all three affected tests — so the hole is precisely and only the type annotation, which is exactly what #58 asked for.

Fix: bring test into a typecheck tsconfig, or add a vitest --typecheck / expectTypeOf assertion on Sender.intColumn.

---

Coverage map

Test gate: pass. One admitted gap (M3, moderate); no critical gaps.

Change	Test	Failure link	Disposition
bigint accepted, ±, v1 wire bytes	"supports BigInt values in integer fields"	exact ILP line; fails in revert tree with Value must be an integer, received 42	covered
2^63-1 without precision loss	"keeps full LONG precision above Number.MAX_SAFE_INTEGER"	exact ILP line; fails in revert tree	covered
changed error message	"throws exception if a float is passed as integer field" (updated)	toThrow string; fails in revert tree	covered
widened Sender.intColumn type	none	narrowing to number keeps every gate green	moderate gap (M3)

Both new-test ILP literals were recomputed by hand from writeColumn + SenderBufferV1.writeTimestamp (timestampToNanos(1658484769000000n, "us") → 1658484769000000000, no t suffix for a designated timestamp) and match character for character. bufferContent observes the same [0, endOfLastRow) range the transport sends, and would throw on null rather than pass vacuously if the row were not closed.

---

Notes

• Checked and found clean: capacity math (checkCapacity([valueStr], 1) reserves exactly byteLength(valueStr) + 1, and BigInt.prototype.toString() is ASCII-only with no exponent form, so Buffer.write truncation is unreachable); buffer growth for multi-thousand-digit bigints; interface widening against external implements SenderBuffer (method parameters are bivariant, so no break); protocol v1/v2/v3 parity, verified by execution rather than inferred; no code anywhere matches on the old error string.
• The changed error message affects every rejected value, not only bigints, but nothing in src/, test/, examples/, or README.md matches on it, and the new wording aligns with timestampColumn. README and examples never mention intColumn, so no doc updates are needed there.
• eslint, prettier --check and tsc --noEmit are clean at head; 174/174 tests pass.
• Minor description nit: the "one pre-existing undici FormData error" from tsc --noEmit did not reproduce here — tsc is clean at both base and head. Likely an @types/node difference; it alleges nothing about the branch, so it is not blocking.

[glasstiger]

Heads-up: this branch now conflicts with main (#62 merged)

#62 landed as da5c1ef, which both moved src/** → packages/nodejs-client/src/** and changed intColumn itself. GitHub reports mergeable: CONFLICTING / mergeStateStatus: DIRTY.

Test-merging origin/main onto a9aecfb in a throwaway worktree:

Auto-merging  packages/nodejs-client/src/buffer/base.ts
CONFLICT (content):        packages/nodejs-client/src/buffer/base.ts
Auto-merging  packages/nodejs-client/src/buffer/index.ts
CONFLICT (content):        packages/nodejs-client/src/buffer/index.ts
CONFLICT (modify/delete):  src/sender.ts deleted in origin/main, modified in HEAD
Auto-merging  test/sender.buffer.test.ts

Three conflicted paths, but the marker-free cases are the ones worth watching.

---

1. packages/nodejs-client/src/buffer/base.ts — content conflict (1 hunk)

Rename-detected from src/buffer/base.ts. This is a genuine semantic collision, not just the path move — #62 rewrote the same lines to add null/undefined column omission (#28):

<<<<<<< HEAD:src/buffer/base.ts
  intColumn(name: string, value: number | bigint): SenderBuffer {
    if (typeof value !== "bigint" && !Number.isInteger(value)) {
      throw new Error(`Value must be an integer or BigInt, received ${value}`);
=======
  intColumn(name: string, value: number | null | undefined): SenderBuffer {
    this.validateColumnCall(name);
    // A null or undefined value omits the column entirely (see issue #28).
    if (this.isNullOrUndefined(value)) {
      return this.omitColumn();
    }
    if (!Number.isInteger(value)) {
      throw new Error(`Value must be an integer, received ${value}`);
>>>>>>> origin/main:packages/nodejs-client/src/buffer/base.ts

Resolution needs both: value: number | bigint | null | undefined, main's validateColumnCall + isNullOrUndefined/omitColumn prologue, then this PR's typeof value !== "bigint" && !Number.isInteger(value) guard. Order matters — the nullish check must stay ahead of the integer guard, otherwise null starts throwing again and #28 regresses.

2. packages/nodejs-client/src/buffer/index.ts — content conflict (2 hunks)

Same collision on the interface — the @param line and the declaration:

<<<<<<< HEAD    @param value - Column value, accepts integer or `BigInt` values. …
=======         @param value - Column value, accepts only number values. A null or undefined value omits the column entirely (stored as NULL).

<<<<<<< HEAD    intColumn(name: string, value: number | bigint): SenderBuffer;
=======         intColumn(name: string, value: number | null | undefined): SenderBuffer;

3. src/sender.ts — modify/delete ⚠️ the one that can silently lose work

Git did not detect the rename to packages/nodejs-client/src/sender.ts (#62 rewrote that file heavily; it now dispatches between a QWP sender and the ILP buffer). Consequences:

• this PR's edit is stranded at the dead path src/sender.ts, left in the tree as UD;
• packages/nodejs-client/src/sender.ts is not conflicted and silently keeps main's version:

// packages/nodejs-client/src/sender.ts:462
intColumn(name: string, value: number | null | undefined): Sender {
  if (this.qwpSender) this.qwpSender.intColumn(name, value);
  else this.buffer!.intColumn(name, value);

Resolving with git rm src/sender.ts — the obvious move — drops the facade change without any warning. The widening has to be hand-applied at packages/nodejs-client/src/sender.ts:453-462 (signature plus the JSDoc block), or Sender.intColumn will reject bigint at the type level while the buffer underneath accepts it.

---

Auto-merged but left failing (no markers, easy to miss)

test/sender.buffer.test.ts merged cleanly with this PR's content winning: the updated assertion at line 1589 ("Value must be an integer or BigInt, received 123.222") and both new tests at 1593 and 1611 are present. Against main's unresolved base.ts those three tests fail. That is the correct signal rather than a defect, but nothing surfaces it during conflict resolution.

New surface from #62 that this PR does not yet cover

Sender.intColumn is now a two-path dispatch and only the ILP path is in this diff:

// packages/client-core/src/_qwp/sender.ts:1502
intColumn(name: string, value: number | null | undefined): QwpSender {
  if (this.omitsNullish(name, value)) return this;
  return this.addColumn(name, QWP_COLUMN_TYPE.LONG,
    BigInt(checkedInteger(value, "intColumn value")));   // checkedInteger(value: number, …)

That path already converts to BigInt internally, but checkedInteger (packages/client-core/src/_qwp/sender.ts:321) is typed (value: number, …), so a bigint argument would fail there while succeeding on ILP. Worth widening both so the two transports agree — otherwise intColumn(x, 42n) works on ILP and throws on QWP.

intColumn also has no entry in test/qwp/public-api-contract.ts today; worth checking whether the contract test is expected to cover it once the QWP signature changes.

---

Suggested resolution order

1. Merge or rebase onto origin/main.
2. Resolve the two packages/nodejs-client/src/buffer/* hunks by combining both changes → number | bigint | null | undefined, nullish check first.
3. git rm src/sender.ts, then re-apply the signature + JSDoc widening to packages/nodejs-client/src/sender.ts:453-462.
4. Widen QwpSender.intColumn and checkedInteger for bigint, or explicitly document that BigInt is ILP-only.
5. Re-run the buffer tests; add a QWP-path bigint test if step 4 is taken.
6. While editing base.ts, the stale TSDoc from M1 in my earlier review is still present on main (@param {number} … accepts only number values) — worth fixing in the same edit.

[glasstiger]

Hi @PedroHenrique0713, the change looks good, many thanks for the contribution!

However, I think M1 and M2 issues (stale docs and missing range check) should be addressed before merge.
See the review: #63 (comment)

Also, a rather big change has just been merged, and there are conflicts on the PR.
I posted a comment on the PR which should help you to resolve the conflicts:
#63 (comment)

[PedroHenrique0713]

Rebased onto main (da5c1ef), which moved the client into packages/nodejs-client and brought the QWP path in, and addressed M1 and M2.

M1 — packages/nodejs-client/src/buffer/base.ts now carries the same @param/@throws wording as the interface and Sender, so the three blocks agree.

M2 — the guard now rejects a BigInt outside int64: intColumn("a", 2n ** 63n) throws Value must fit into a 64-bit signed integer, received 9223372036854775808 instead of putting 9223372036854775808i on the wire. The bounds themselves stay in range, so -9223372036854775808n still writes — that is QuestDB's LONG NULL sentinel, the value QwpSender.longColumn() documents, and the test says so. Say the word if you would rather reject it here. The pre-existing number holes you noted (2 ** 63, 1e21) are untouched; happy to take them in a separate PR.

M3 — no longer reproducible on main: tsconfig.test.json includes test and pnpm typecheck:test is a CI step. Narrowing Sender.intColumn back to number on top of this branch fails it with six TS2345 errors, three of them the ones you quoted.

One thing the rebase forced a decision on: Sender.intColumn() delegates to QwpSender.intColumn() when the QWP transport is active, and that one validated with checkedInteger(), so a BigInt would have thrown there while the ILP path accepted it. It now uses checkedInt64() — the helper longColumn() already uses for the same QWP LONG column — so numbers keep the safe-integer rule and BigInts get the int64 bound. Tell me if you would rather keep this PR to the ILP buffer and leave QWP to its own change.

Verified locally: typecheck, typecheck:test, eslint and format:check clean; pnpm test 1093/1094. The one failure is test/qwp/egress.test.ts > bounds the grid a RESULT_BATCH declares timing out at 5s, which also runs 3.5-4.6s on unpatched main on this machine — my load, not this diff.