Skip to content

Key recursive CTE UNION rows by value type to close #2881 - #2940

Draft
kuldeepsinh19 wants to merge 1 commit into
AlaSQL:developfrom
kuldeepsinh19:fix-2881-recursive-cte-union-keys
Draft

kuldeepsinh19 wants to merge 1 commit into
AlaSQL:developfrom
kuldeepsinh19:fix-2881-recursive-cte-union-keys

Conversation

@kuldeepsinh19

Copy link
Copy Markdown

Fixes #2881

Problem

A recursive CTE with UNION (not UNION ALL) throws on BigInt, and counts NaN and NULL, or a Date and its ISO string, as one row:

var q = 'WITH RECURSIVE r(n) AS (SELECT n FROM ? UNION SELECT n FROM r WHERE 1 = 0) SELECT COUNT(*) AS c FROM r';
alasql(q, [[{n: 1n}, {n: 2n}]]);   // TypeError: Do not know how to serialize a BigInt
alasql(q, [[{n: NaN}, {n: null}]]);   // [{c: 1}], expected [{c: 2}]
alasql(q, [[{n: new Date('2020-01-01T00:00:00.000Z')}, {n: '2020-01-01T00:00:00.000Z'}]]);   // [{c: 1}], expected [{c: 2}]

Cause

Since #2697, executeRecursiveCTE in src/67withselect.js (the issue mentions src/statements/with.js, which isn't on develop) builds the key of each row for the UNION check with JSON.stringify. JSON.stringify throws on BigInt, writes NaN as null, and writes a Date as its ISO string.

Fix

A new cteValueKey helper in the same file builds the key of each column value with its type, and the row key is the JSON of those value keys:

  • null and undefined are both NULL, as before
  • a Date gives its time, so a Date and its ISO string stay apart
  • a boxed String or Number is unwrapped with getValueOf, so it still matches the plain value, as before
  • any other primitive gives its typeof plus String(value), so NaN equals NaN (a cycle through NaN still ends), and 1, 1n and '1' stay apart
  • a value inside an object is still compared as JSON (BigInt no longer throws there)

UNION ALL doesn't change.

Existing helpers weren't reused: deepEqual (used by plain UNION) treats any two Dates as equal, treats NaN as not equal to NaN, and compares every pair of rows. distinctArray writes every value as a string, so 1, 1n and '1' collide.

Tests

New test/test2881.js:

  • A, B, C: the three queries from the issue
  • D: equal numbers, NULL and undefined, and equal Dates are still added only once
  • E: equal BigInts and equal NaNs are added only once
  • F: a recursive step that only finds rows already in the result ends the recursion (BigInt, NaN and a Date)
  • G: a boxed string is the same value as the plain string

A, B, C, E and F fail without the change in src/67withselect.js and pass with it. D and G pass both ways. The cycle tests in test/test2273-B.js still pass.

Full suite with Bun 1.4.4 on Node 24: 2531 pass, 213 skip, 11 fail. The 11 failures are the tests that load files from the internet, which had no network access here. The same 11 fail without this change. prettier --list-different is clean on the changed files.

Not in this PR

Two related problems, left for separate changes:

  • DISTINCT ON builds its key with JSON.stringify too (src/424select.js): SELECT DISTINCT ON (n) n FROM ? with [{n: NaN}, {n: null}] returns 1 row, and with BigInt values it throws.
  • Plain UNION keeps only one of two different Dates: SELECT d FROM ? UNION SELECT d FROM ? with [{d: new Date(0)}] and [{d: new Date(86400000)}] returns 1 row, because deepEqual treats any two Dates as equal.

This change was written with help from an AI coding assistant (Claude Code).

A recursive CTE with UNION built the key of each row with JSON.stringify,
which throws on BigInt and counts NaN and NULL, or a Date and its ISO
string, as the same row. Each column value is now keyed with its type.

Fixes AlaSQL#2881
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Recursive CTE with UNION: BigInt throws, and NaN and NULL, or a Date and its ISO string, count as one row

1 participant