From 55d584b30930dd8e11f317c8005211764c6ffde4 Mon Sep 17 00:00:00 2001 From: test Date: Mon, 17 Aug 2026 22:37:59 +0000 Subject: [PATCH 1/3] Union absent-column tests pin the pre-LLP-0241 contract, so master is red (#820) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four tests in `test/core/union-source.test.js` fail on `master`, and because GitHub's `pull_request` runs build the branch merged into the base, every open PR inherits the failure: the drifted cell is unresolved and throws; only collect() turns it into undefined a partition whose rows carry no resolved map makes a bare projection throw evaluating a column one partition lacks throws, and so does a non-identifier sibling SELECT * keeps each partition row shape, so a drifted key is absent rather than undefined This is a semantic conflict between two changes that were each green alone. `192d3f9e` (#789) landed LLP 0241, which changed runtime behaviour: a scan's rows now carry the column list the scan advertised, so `unionSources` pads a partition that physically lacks a column with a real cell resolving to `undefined`. `70b9c1c7` (#740) landed afterwards but was cut before it, and is doc-and-tests only ("No runtime behaviour changes"). Its tests describe the tree as it stood before the padding. The tests are what is stale. LLP 0241 is Accepted and settles every one of the four behaviours in the padded direction, by name, in its own Consequences section: - "A padded cell resolves to `undefined`" and it is a cell, not the unresolved throwing thunk the first two tests inspect. - "A query whose `WHERE` or `ORDER BY` names a column some partition lacks stops throwing `ColumnNotFoundError` and answers", which is the third test. - "`Object.keys(row).length` for a star over a drifted partition now equals the declared column count rather than the physical one", which is the fourth. 0241 also states why that direction is the intended one rather than a regression: it is "the behaviour LLP 0015 already required of a union ('projecting an absent column reads as null, never throws'); the throw was the same short row surfacing on a different path". Satisfying #740's tests would mean reverting an Accepted decision's implementation, which is the wrong fix. Measured on the drifted two-partition parquet fixture the tests already build, current tree: `SELECT extra FROM t` gives `resolvedHasKey: true` and a cell resolving to `undefined` on both narrow rows; `WHERE extra = 'x'`, `ORDER BY extra`, `max(extra)`, `coalesce(extra, 'none')` and `SELECT extra, 1 AS n` all answer instead of throwing; and `SELECT *` yields keys `[id, score, extra]` on every row while still rendering `[{"id":1,"score":1.5},...]`, because `JSON.stringify` drops `undefined` exactly as it dropped the missing key. So: - Rewrite the four tests against the post-0241 contract, keeping each one's coverage intent (the cell mechanism, a hand-rolled source with no `resolved` map, the evaluating and non-identifier-sibling shapes, and the star) and repointing their `@ref`s at LLP 0241 §alignment. - Replace the now-false absent-column paragraph in the `unionSources` header comment. It described the same pre-0241 tree. - Correct the same paragraph in LLP 0015's "Multi-partition union", which already carried the `Extended-by: LLP 0241 §alignment` forward-ref pointing at the behaviour its prose contradicted, and record the second correction inline the way the first one was. No runtime behaviour changes. The three neutrally-worded ai-gateway comments #740 left ("the exact value depends on the read path") are still true and are untouched. --- llp/0015-query-and-datasets.spec.md | 61 +++++++------ src/core/query/union-source.js | 38 ++++---- test/core/union-source.test.js | 135 ++++++++++++++++++---------- 3 files changed, 131 insertions(+), 103 deletions(-) diff --git a/llp/0015-query-and-datasets.spec.md b/llp/0015-query-and-datasets.spec.md index 17a88ce9b..049595c1b 100644 --- a/llp/0015-query-and-datasets.spec.md +++ b/llp/0015-query-and-datasets.spec.md @@ -74,41 +74,40 @@ column a given partition physically lacks, and a parquet-backed source throws partition can't satisfy the predicate the union drops `where` for it and lets the engine filter. `columns` is always forwarded, which adds no failure the merged stream did not already have, but an absent column does **not** read as -null. A bare identifier projection (`SELECT extra FROM t`, with or without an -alias, a `LIMIT`, or a predicate on a column every partition has) leaves the -drifted cell **unresolved**: `executeProject` finds no matching cell, so it emits -a lazy `evaluateExpr` thunk that would throw, and writes no entry into the row's -`resolved` map. `collect()` reads the pre-materialized `resolved` map for every -advertised column and never invokes that thunk, so the key is present with the -value `undefined`, is not `null`, and `JSON.stringify` drops it. This holds only -under three conditions, all of them in `executeProject` and `collect()` rather -than in the union: the consumer goes through `collect()` (as `executeQuerySql` -does); every row reaching `collect()` carries a `resolved` map, which holds here -because every in-repo partition's rows come from squirreling's `asyncRow`, the -only thing that pre-materializes one; **and every output column of that -projection is a star or a bare identifier**. `executeProject` gates on the last -one up front (`resolveable`) and emits no `resolved` map at all when it fails, so -a single non-identifier sibling column (an expression, a function, or even a -literal) collapses the fast path for the whole result and the drifted column's -thunk is invoked and throws: `SELECT extra, 1 AS n FROM t` throws even though -nothing evaluates `extra`. A row that carries no `resolved` map at all does the -same. Anything -that *evaluates* the absent column throws squirreling's `ColumnNotFoundError` at -the first row from a partition without it: a `WHERE` on it, an expression or -function over it, `ORDER BY`, `GROUP BY`, `DISTINCT`, or an aggregate on it. -`SELECT *` is unaffected: each partition's rows keep their own shape, so the key -is simply absent. The column is addressable at all only because the union -advertises the superset of partition columns; when no partition has it, planning -fails with the same error unless a wrapper advertises the declared schema on top -of the union (LLP 0032's `withSchemaColumns`), which keeps such a column -addressable; the exact value a read of it then yields depends on the read path -and is not settled here. Pinned by +null: it reads as `undefined`. The union pads every row out to the column list +the scan advertised +([LLP 0241 §alignment](./0241-scan-rows-carry-advertised-columns.decision.md#alignment)), +so a column a partition physically lacks is still a real cell on the row, and +that cell resolves to `undefined`. The value is outside `SqlPrimitive` and +`JSON.stringify` drops it, so a padded column renders as an absent key even +though the row object owns it, and `Object.keys(row).length` over a star counts +the **advertised** columns rather than the physical ones. Every read path agrees +on that value: reading the row's pre-materialized `resolved` map (`collect()`'s +fast path), invoking the cell directly, and evaluating the column above the scan +in a `WHERE`, an expression or function over it, `ORDER BY`, `GROUP BY`, +`DISTINCT`, or an aggregate. `SELECT *` renders identically to the unpadded +star, because the key it now owns is one `JSON.stringify` drops. The column is +addressable at all only because the union advertises the superset of partition +columns; when no partition has it, planning fails unless a wrapper advertises +the declared schema on top of the union (LLP 0032's `withSchemaColumns`), which +keeps such a column addressable; the exact value a read of it then yields +depends on the read path and is not settled here. Pinned by [`test/core/union-source.test.js`](../test/core/union-source.test.js). > **Corrected (#731, PR #740).** This section previously stated that projecting > an absent column "reads as null, never throws". That was never true of the -> code; the paragraph above records the measured contract. No runtime behaviour -> changed. +> code when it was written; the paragraph above records the measured contract. +> No runtime behaviour changed. + +> **Corrected again (#820).** PR #740 was cut before, and merged after, +> [LLP 0241](./0241-scan-rows-carry-advertised-columns.decision.md), so the text +> it landed described a tree that no longer existed: an unresolved drifted cell, +> `undefined` only via `collect()`'s pre-materialized fast path, and a +> `ColumnNotFoundError` from anything that evaluated the column or merely sat +> beside it as a non-identifier sibling. 0241's padding removed that seam, and +> the semantic conflict turned `master` red. The paragraph above records the +> contract as measured on the current tree. No runtime behaviour changed in this +> correction either. > **Extended-by: [LLP 0241 §alignment](./0241-scan-rows-carry-advertised-columns.decision.md#alignment).** > The hints above say what a union may forward; LLP 0241 adds what shape the diff --git a/src/core/query/union-source.js b/src/core/query/union-source.js index f9f09abde..d833e00ba 100644 --- a/src/core/query/union-source.js +++ b/src/core/query/union-source.js @@ -102,29 +102,21 @@ export async function* alignRows(rows, columns) { * reading the column as null. When a partition can't satisfy the predicate we * drop `where` for it and let the engine filter the concatenated stream (it * already owns the filter via `appliedWhere: false`). `columns` is always - * forwarded, which adds no failure the merged stream did not already have, but - * an absent column does NOT read as null. A bare identifier projection leaves - * the drifted cell unresolved: squirreling's `executeProject` finds no matching - * cell, so it emits a lazy `evaluateExpr` thunk that would throw and writes no - * entry into the row's `resolved` map. `collect()` reads the pre-materialized - * `resolved` map for every advertised column and never invokes that thunk, so - * the key is present with the value `undefined` (not `null`, dropped by - * `JSON.stringify`). That holds only under three conditions, all of them in - * `executeProject` and `collect()` rather than in the union: the consumer goes - * through `collect()`; every row reaching `collect()` carries a `resolved` map, - * which holds here because every in-repo partition's rows come from - * squirreling's `asyncRow`, the only thing that pre-materializes one; and every - * output column of that projection is a star or a bare identifier. - * `executeProject` gates on the last one up front (`resolveable`) and emits no - * `resolved` map at all when it fails, so a single non-identifier sibling column - * (an expression, a function, or even a literal) collapses the fast path for the - * whole result and the drifted column's thunk is invoked and throws: - * `SELECT extra, 1 AS n FROM t` throws even though nothing evaluates `extra`. A - * row that carries no `resolved` map at all does the same. - * Anything that evaluates the column (a `WHERE` on it, an expression over it, - * `ORDER BY`/`GROUP BY`/`DISTINCT`, an aggregate) throws `ColumnNotFoundError` - * at the first such row. `SELECT *` keeps each partition's own row shape, so the - * key is simply absent. `test/core/union-source.test.js` pins both halves. + * forwarded, which adds no failure the merged stream did not already have, and + * an absent column reads as `undefined` rather than `null` or a throw: every + * row is padded out to the scan's advertised column list by `alignRows` below, + * so a column a partition physically lacks is still a real cell that resolves + * to `undefined` (LLP 0241 §alignment). One consequence is worth stating, + * because it is the thing a maintainer gets wrong: `undefined` is outside + * `SqlPrimitive` and `JSON.stringify` drops it, so a padded column renders as + * an absent key even though the row object owns it. `Object.keys(row).length` + * over a star therefore counts the advertised columns, not the physical ones, + * and is not a way to discover what a partition holds. What no longer varies is + * which read path the caller took: reading `resolved`, invoking the cell, and + * evaluating the column in a `WHERE`, `ORDER BY`, `GROUP BY`, `DISTINCT` or an + * aggregate all agree, where a short row made the last group throw + * `ColumnNotFoundError` on the first partition lacking the column. + * `test/core/union-source.test.js` pins these. * * Because a sub-source now emits exactly the columns it is asked for (see * `parquet-source.js`), forwarding `columns` also determines what the engine diff --git a/test/core/union-source.test.js b/test/core/union-source.test.js index 1814f119c..2e0d09e0a 100644 --- a/test/core/union-source.test.js +++ b/test/core/union-source.test.js @@ -483,7 +483,7 @@ function hasExtraKey(rows, key = 'extra') { return rows.map((row) => Object.prototype.hasOwnProperty.call(row, key)) } -// @ref LLP 0015#multi-partition-union [tests]: forwarding `columns` does not null-pad; the drifted cell is `undefined`, which is why the doc no longer promises null +// @ref LLP 0015#multi-partition-union [tests]: the union pads a drifted cell with `undefined`, never `null`, which is why the doc no longer promises null test('a projected column one partition lacks reads as undefined, never null', async () => { // Every bare-projection shape the LLP names, so "Pinned by" covers the alias // and `LIMIT` variants it claims and not just the plain projection. @@ -492,8 +492,9 @@ test('a projected column one partition lacks reads as undefined, never null', as { query: 'SELECT extra FROM t WHERE score > 1', key: 'extra', values: [undefined, undefined, 'x'], json: '[{},{},{"extra":"x"}]' }, { query: 'SELECT extra AS e FROM t', key: 'e', values: [undefined, undefined, 'x'], json: '[{},{},{"e":"x"}]' }, // A bare-identifier SIBLING keeps `executeProject`'s `resolveable` gate - // open, so the fast path survives. The throwing test below pins what a - // non-identifier sibling does to the very same query. + // open, so `collect()` stays on its pre-materialized fast path. The test + // below pins the same answer coming back off the slow path, where a + // non-identifier sibling closes that gate and the cell is really invoked. { query: 'SELECT score, extra FROM t', key: 'extra', values: [undefined, undefined, 'x'], json: '[{"score":1.5},{"score":2.5},{"score":3.5,"extra":"x"}]' }, { query: 'SELECT extra FROM t LIMIT 1', key: 'extra', values: [undefined], json: '[{}]' }, { query: 'SELECT extra FROM t LIMIT 0', key: 'extra', values: [], json: '[]' }, @@ -503,35 +504,43 @@ test('a projected column one partition lacks reads as undefined, never null', as assert.equal(rows.length, values.length, query) assert.deepEqual(hasExtraKey(rows, key), values.map(() => true), 'the projection puts the key on every row') assert.deepEqual(rows.map((row) => row[key]), values, query) - assert.equal(rows[0]?.[key] === null, false, 'undefined, not null: the union never pads the drifted partition') + assert.equal(rows[0]?.[key] === null, false, 'undefined, not null: the union pads the drifted cell, it does not null it') assert.equal(JSON.stringify(rows), json, 'JSON.stringify drops the undefined cells') } }) -// @ref LLP 0015#multi-partition-union [tests]: the undefined read is `collect()`'s pre-materialized fast path, not a copied value; the cell itself throws -test('the drifted cell is unresolved and throws; only collect() turns it into undefined', async () => { +// @ref LLP 0241#alignment [tests]: the drifted cell is padded by the union, so it is a real resolvable cell rather than a throwing thunk +test('the drifted cell is padded, so it resolves to undefined instead of throwing', async () => { const results = executeSql({ tables: { t: await driftedUnion() }, query: 'SELECT extra FROM t' }) - /** @type {{ resolvedHasKey: boolean, cell: string }[]} */ + /** @type {{ resolvedHasKey: boolean, cell: string, value: SqlPrimitive | undefined }[]} */ const seen = [] for await (const row of results.rows()) { const resolved = row.resolved let cell = 'ok' + /** @type {SqlPrimitive | undefined} */ + let value try { - await row.cells.extra() + value = await row.cells.extra() } catch (err) { cell = err instanceof Error ? err.constructor.name : 'unknown' } - seen.push({ resolvedHasKey: !!resolved && Object.prototype.hasOwnProperty.call(resolved, 'extra'), cell }) + seen.push({ resolvedHasKey: !!resolved && Object.prototype.hasOwnProperty.call(resolved, 'extra'), cell, value }) } + // The union pads each row out to the column list the scan advertised, so + // `executeProject` finds `extra` in `row.cells` on every row and takes its + // copy path. The cell is real, it resolves to `undefined`, and `resolved` + // carries the key. Nothing here depends on `collect()`'s fast path: the + // caller reading the cell directly and the caller reading `resolved` now + // agree, which is what stops the same query answering two different ways. assert.deepEqual(seen, [ - { resolvedHasKey: false, cell: 'ColumnNotFoundError' }, - { resolvedHasKey: false, cell: 'ColumnNotFoundError' }, - { resolvedHasKey: true, cell: 'ok' }, - ], 'a drifted cell is a throwing thunk with no `resolved` entry; `collect()` reads `resolved` and never calls it') + { resolvedHasKey: true, cell: 'ok', value: undefined }, + { resolvedHasKey: true, cell: 'ok', value: undefined }, + { resolvedHasKey: true, cell: 'ok', value: 'x' }, + ], 'the padded cell resolves to undefined and `resolved` carries the key') }) -// @ref LLP 0015#multi-partition-union [tests]: the undefined read is load-bearing on every partition pre-materializing `resolved` -test('a partition whose rows carry no resolved map makes a bare projection throw', async () => { +// @ref LLP 0241#alignment [tests]: padding happens below `executeProject`, so a partition that pre-materializes no `resolved` map reads the same as one that does +test('a partition whose rows carry no resolved map reads the same, because the union pads below it', async () => { /** * A legal `AsyncDataSource` that hand-rolls its rows instead of going * through squirreling's `asyncRow`, so nothing pre-materializes `resolved`. @@ -568,45 +577,73 @@ test('a partition whose rows carry no resolved map makes a bare projection throw unresolvedSource(['id', 'score'], [{ id: 1, score: 1.5 }]), unresolvedSource(['id', 'score', 'extra'], [{ id: 3, score: 3.5, extra: 'x' }]), ]) - for (const query of ['SELECT extra FROM t', 'SELECT extra AS e FROM t', 'SELECT extra FROM t LIMIT 1']) { - await assert.rejects( - () => collect(executeSql({ tables: { t: union }, query })), - /Column "extra" not found/, - `${query} throws without collect()'s pre-materialized fast path` - ) - } -}) - -// @ref LLP 0015#multi-partition-union [tests]: evaluating an absent column throws rather than reading as null, and so does a non-identifier sibling column that never touches it -test('evaluating a column one partition lacks throws, and so does a non-identifier sibling', async () => { - const evaluating = [ - 'SELECT extra FROM t WHERE extra IS NOT NULL', - "SELECT id FROM t WHERE extra = 'x'", - "SELECT coalesce(extra, 'none') AS e FROM t", - 'SELECT id, extra FROM t ORDER BY extra', - 'SELECT max(extra) AS m FROM t', + // The union pads before `executeProject` ever sees a row, so where the + // source got its cells no longer decides the answer. A hand-rolled source + // that never calls squirreling's `asyncRow` reads exactly like the parquet + // partitions above. + /** @type {[string, string, (SqlPrimitive | undefined)[]][]} */ + const cases = [ + ['SELECT extra FROM t', 'extra', [undefined, 'x']], + ['SELECT extra AS e FROM t', 'e', [undefined, 'x']], + ['SELECT extra FROM t LIMIT 1', 'extra', [undefined]], + // A non-identifier sibling closes `executeProject`'s `resolveable` gate, + // so `collect()` falls off its pre-materialized fast path and invokes the + // cell. The padded cell resolves, so this shape answers too. + ['SELECT extra, 1 AS n FROM t', 'extra', [undefined, 'x']], ] - for (const query of evaluating) { - await assert.rejects(() => runDrifted(query), /Column "extra" not found/, query) + for (const [query, key, values] of cases) { + const rows = await collect(executeSql({ tables: { t: union }, query })) + assert.deepEqual(hasExtraKey(rows, key), values.map(() => true), `${query} puts the key on every row`) + assert.deepEqual(rows.map((row) => row[key]), values, query) } +}) - // Nothing below evaluates `extra`. `executeProject` computes `resolveable` - // over the WHOLE column list and emits no `resolved` map when any output - // column is neither a star nor a bare identifier, so one literal or one - // expression sibling drops `collect()`'s fast path for the entire result, - // the drifted thunk is invoked, and it throws. This is the shape that - // surprises people: `SELECT score, extra FROM t` above reads `undefined`, - // and adding `, 1 AS n` to it throws. - const siblingCollapsesTheFastPath = [ - 'SELECT extra, 1 AS n FROM t', - 'SELECT extra, score * 2 AS d FROM t', - ] - for (const query of siblingCollapsesTheFastPath) { - await assert.rejects(() => runDrifted(query), /Column "extra" not found/, query) +// @ref LLP 0241#alignment [tests]: a clause the engine evaluates above the scan reads the padded cell as undefined and answers, where a short row made it throw +test('evaluating a column one partition lacks answers with undefined, and so does a non-identifier sibling', async () => { + // Every shape here read the absent column off `row.cells` above the scan and + // raised `ColumnNotFoundError` on the short row. On a padded row the lookup + // hits, resolves to `undefined`, and the query answers. The answers are the + // ones the hinted form of each query always gave, so this is the union + // agreeing with itself rather than a new result. + assert.deepEqual(await runDrifted('SELECT extra FROM t WHERE extra IS NOT NULL'), [{ extra: 'x' }]) + assert.deepEqual((await runDrifted("SELECT id FROM t WHERE extra = 'x'")).map((row) => Number(row.id)), [3]) + assert.deepEqual(await runDrifted("SELECT coalesce(extra, 'none') AS e FROM t"), [{ e: 'none' }, { e: 'none' }, { e: 'x' }]) + assert.deepEqual(await runDrifted('SELECT max(extra) AS m FROM t'), [{ m: 'x' }]) + + const ordered = await runDrifted('SELECT id, extra FROM t ORDER BY extra') + assert.deepEqual(ordered.map((row) => Number(row.id)), [1, 2, 3]) + assert.deepEqual(ordered.map((row) => row.extra), [undefined, undefined, 'x']) + + // Nothing below evaluates `extra`, but a literal or expression sibling + // closes `executeProject`'s `resolveable` gate, so `collect()` leaves its + // pre-materialized fast path and invokes the cell for real. The padded cell + // resolves, so the sibling no longer decides whether the query answers. + for (const [query, key, values] of /** @type {[string, string, (SqlPrimitive | undefined)[]][]} */ ([ + ['SELECT extra, 1 AS n FROM t', 'n', [1, 1, 1]], + ['SELECT extra, score * 2 AS d FROM t', 'd', [3, 5, 7]], + ])) { + const rows = await runDrifted(query) + assert.deepEqual(rows.map((row) => row.extra), [undefined, undefined, 'x'], query) + assert.deepEqual(rows.map((row) => row[key]), values, query) } }) -test('SELECT * keeps each partition row shape, so a drifted key is absent rather than undefined', async () => { +// @ref LLP 0241#alignment [tests]: a star pads each partition row out to the advertised list, so the key count matches the schema `QueryResults.columns` already promised +test('SELECT * pads each partition row to the union column list, so a drifted key is present and undefined', async () => { const rows = await runDrifted('SELECT * FROM t') - assert.deepEqual(hasExtraKey(rows), [false, false, true], 'a star projection copies only the columns a row has') + assert.deepEqual(hasExtraKey(rows), [true, true, true], 'every row carries every advertised column') + assert.deepEqual(rows.map((row) => Object.keys(row)), [ + ['id', 'score', 'extra'], + ['id', 'score', 'extra'], + ['id', 'score', 'extra'], + ], 'the key order is the advertised order, on the narrow partition too') + assert.deepEqual(rows.map((row) => row.extra), [undefined, undefined, 'x']) + // The padded cell is `undefined`, not `null`, so the rendering is byte for + // byte what the unpadded star produced: `JSON.stringify` drops the key it + // used to drop by absence. + assert.equal( + JSON.stringify(rows, (_key, value) => (typeof value === 'bigint' ? Number(value) : value)), + '[{"id":1,"score":1.5},{"id":2,"score":2.5},{"id":3,"score":3.5,"extra":"x"}]', + 'padding an absent cell with undefined renders identically to omitting the key' + ) }) From db35cf5298eb53bbf19db4f8ec2f21674dde6acd Mon Sep 17 00:00:00 2001 From: test Date: Mon, 17 Aug 2026 23:05:20 +0000 Subject: [PATCH 2/3] Scope the union's absent-column agreement to the row path (#820 review) LLP 0015's corrected paragraph and the `unionSources` header both said every read path now agrees on `undefined`. The `scanColumn` column-stream path is not part of that agreement: the union forwards each partition's chunks unchanged, ai-gateway's `withSchemaColumns` is what maps the holes to `null` (dataset.js), and LLP 0241 says in as many words that it "does not touch the null/undefined split between the scanColumn and row paths". Pinned in-repo by test/core/ai-gateway-dataset.test.js:323, which asserts strict `null` on that path. As written, LLP 0015's paragraph also contradicted itself: "every read path agrees" two sentences before "the exact value a read of it yields depends on the read path". Scope both statements to the row path and name what the scanColumn path actually does. Co-Authored-By: Claude Opus 5 (1M context) --- llp/0015-query-and-datasets.spec.md | 17 +++++++++++------ src/core/query/union-source.js | 11 ++++++++--- 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/llp/0015-query-and-datasets.spec.md b/llp/0015-query-and-datasets.spec.md index 049595c1b..739ea6d92 100644 --- a/llp/0015-query-and-datasets.spec.md +++ b/llp/0015-query-and-datasets.spec.md @@ -81,12 +81,17 @@ so a column a partition physically lacks is still a real cell on the row, and that cell resolves to `undefined`. The value is outside `SqlPrimitive` and `JSON.stringify` drops it, so a padded column renders as an absent key even though the row object owns it, and `Object.keys(row).length` over a star counts -the **advertised** columns rather than the physical ones. Every read path agrees -on that value: reading the row's pre-materialized `resolved` map (`collect()`'s -fast path), invoking the cell directly, and evaluating the column above the scan -in a `WHERE`, an expression or function over it, `ORDER BY`, `GROUP BY`, -`DISTINCT`, or an aggregate. `SELECT *` renders identically to the unpadded -star, because the key it now owns is one `JSON.stringify` drops. The column is +the **advertised** columns rather than the physical ones. Every **row**-path +read agrees on that value: reading the row's pre-materialized `resolved` map +(`collect()`'s fast path), invoking the cell directly, and evaluating the column +above the scan in a `WHERE`, an expression or function over it, `ORDER BY`, +`GROUP BY`, `DISTINCT`, or an aggregate. The `scanColumn` column-stream path is +**not** part of that agreement and is not what padding changed: the union +forwards each partition's chunks unchanged, and a wrapper above it normalizes +the holes if it wants them uniform (LLP 0032's `withSchemaColumns` maps them to +`null`). LLP 0241 deliberately left that `null`/`undefined` split between the +two paths unsettled. `SELECT *` renders identically to the unpadded star, +because the key it now owns is one `JSON.stringify` drops. The column is addressable at all only because the union advertises the superset of partition columns; when no partition has it, planning fails unless a wrapper advertises the declared schema on top of the union (LLP 0032's `withSchemaColumns`), which diff --git a/src/core/query/union-source.js b/src/core/query/union-source.js index d833e00ba..8cf866923 100644 --- a/src/core/query/union-source.js +++ b/src/core/query/union-source.js @@ -112,11 +112,16 @@ export async function* alignRows(rows, columns) { * an absent key even though the row object owns it. `Object.keys(row).length` * over a star therefore counts the advertised columns, not the physical ones, * and is not a way to discover what a partition holds. What no longer varies is - * which read path the caller took: reading `resolved`, invoking the cell, and + * which ROW path the caller took: reading `resolved`, invoking the cell, and * evaluating the column in a `WHERE`, `ORDER BY`, `GROUP BY`, `DISTINCT` or an * aggregate all agree, where a short row made the last group throw - * `ColumnNotFoundError` on the first partition lacking the column. - * `test/core/union-source.test.js` pins these. + * `ColumnNotFoundError` on the first partition lacking the column. The + * `scanColumn` hook below is a DIFFERENT path and padding does not touch it: it + * forwards each partition's chunks unchanged, so an absent column's value there + * is whatever the partition streams, and a wrapper above the union normalizes + * those holes if it wants them uniform (ai-gateway's `withSchemaColumns` maps + * them to `null`). LLP 0241 left that `null`/`undefined` split between the two + * paths unsettled on purpose. `test/core/union-source.test.js` pins these. * * Because a sub-source now emits exactly the columns it is asked for (see * `parquet-source.js`), forwarding `columns` also determines what the engine From 83674f1d7560a451f7f6b5b99f9625d09755bcc7 Mon Sep 17 00:00:00 2001 From: test Date: Mon, 17 Aug 2026 23:37:38 +0000 Subject: [PATCH 3/3] ai-gateway's scanColumn null is not what the row path reads (#820 review) `withSchemaColumns.scanColumn` normalizes an absent column's `undefined` holes to `null` and its comment justified that as "the same ... value the row path reads". That was written in July, before LLP 0241. Post-0241 the row path pads an absent cell with `undefined`, so the two paths read different values, which is exactly the split this PR just scoped in LLP 0015 and in the `unionSources` header. Keep the real justification (one representation across the merged stream) and name the split instead of asserting sameness. Comment only. No runtime behaviour change. Co-Authored-By: Claude Opus 5 (1M context) --- hypaware-core/plugins-workspace/ai-gateway/src/dataset.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/hypaware-core/plugins-workspace/ai-gateway/src/dataset.js b/hypaware-core/plugins-workspace/ai-gateway/src/dataset.js index 532ae9c9f..e06116945 100644 --- a/hypaware-core/plugins-workspace/ai-gateway/src/dataset.js +++ b/hypaware-core/plugins-workspace/ai-gateway/src/dataset.js @@ -200,8 +200,12 @@ function withSchemaColumns(source) { // engine's streaming fast path. A partition that physically lacks the // requested column (the additive schema-drift case this wrapper exists // for) surfaces its values as `undefined` holes in the chunk; normalize - // them to null, the same "this partition predates the column" value the - // row path reads, so accumulators see one representation either way. + // them to null so every partition's chunk reads the same way and an + // accumulator sees one representation across the merged stream. This is + // NOT the value the row path reads: `scan` above pads an absent cell with + // `undefined` (LLP 0241 §alignment), and 0241 deliberately left the + // null/undefined split between the two paths unsettled, so nothing may + // branch on which one it got. // // A `where` naming a DECLARED-but-physically-absent column can't be // handed to the source: this wrapper is the only layer that knows the