From de5fc920b2d5fb841a385ad87057212e20940893 Mon Sep 17 00:00:00 2001 From: test Date: Sat, 15 Aug 2026 09:13:55 +0000 Subject: [PATCH 1/3] A scan's rows carry the column list the scan advertised (#788) Over a drifted union, `SELECT *, git_remote` returned a row with another column holding git_remote's value. Reproduced first, then explained. The engine derives a query's output column names once, from the scan's advertised list, then fills them by walking each ROW's own `columns` and advancing a shared index. A partition that predates a declared column yields a shorter row, which under-runs the index and slides every output name after the star onto its neighbour's value. Isolated with a hand-rolled source, no icebird and no HypAware in the picture. Fix: align a scan's rows to `options.columns ?? source.columns` at the two places a HypAware row can be narrower than what its source advertises, `unionSources.scan` and the AI-gateway `withSchemaColumns.scan`. A padded cell resolves to `undefined`, the same value the row path already read for an absent column, so no existing query's answer changes and `SELECT *` renders identically. Co-Authored-By: Claude --- .../ai-gateway/src/dataset.js | 18 +- llp/0015-query-and-datasets.spec.md | 7 + ...-rows-carry-advertised-columns.decision.md | 136 ++++++++++ src/core/query/index.js | 2 +- src/core/query/union-source.js | 87 ++++++- .../core/star-expansion-drifted-union.test.js | 244 ++++++++++++++++++ 6 files changed, 487 insertions(+), 7 deletions(-) create mode 100644 llp/0241-scan-rows-carry-advertised-columns.decision.md create mode 100644 test/core/star-expansion-drifted-union.test.js diff --git a/hypaware-core/plugins-workspace/ai-gateway/src/dataset.js b/hypaware-core/plugins-workspace/ai-gateway/src/dataset.js index dd8cf192..2d14e88a 100644 --- a/hypaware-core/plugins-workspace/ai-gateway/src/dataset.js +++ b/hypaware-core/plugins-workspace/ai-gateway/src/dataset.js @@ -5,7 +5,7 @@ import path from 'node:path' import { discoverCachePartitions } from '../../../../src/core/cache/partition.js' import { isUsagePolicyDrop } from '../../../../src/core/usage-policy/index.js' -import { canPushWhere, emptySource, normalizeScanColumn, unionSources, whereColumns } from 'hypaware/core/query' +import { alignRows, canPushWhere, emptySource, normalizeScanColumn, unionSources, whereColumns } from 'hypaware/core/query' import { AI_GATEWAY_MESSAGE_COLUMNS, aiGatewayRowsFromProjectedExchange } from './message_projector.js' import { isPlainObject, stringValue } from 'hypaware/core/util' @@ -178,7 +178,21 @@ function withSchemaColumns(source) { columns, numRows: source.numRows, scan(options) { - return source.scan(options) + // The engine names this scan's output columns from the list advertised + // here, but fills them from each row's own `columns`. A partition that + // predates a declared column yields a SHORTER row, which slides every + // output name past the gap onto its neighbour's value: over a drifted + // union `SELECT *, git_remote` answered with git_remote's value under + // the name of the column that happened to follow the star's short + // width. Pad each row back out to the advertised list. + // @ref LLP 0241#alignment [implements]: a declared-but-absent column becomes a padded cell, not a missing slot the star can slide through + const scanColumns = options?.columns ?? columns + const result = source.scan(options) + return { + appliedWhere: result.appliedWhere, + appliedLimitOffset: result.appliedLimitOffset, + rows: () => alignRows(result.rows(), scanColumns), + } }, } // Forward the column-stream hook so single-column aggregates stay on the diff --git a/llp/0015-query-and-datasets.spec.md b/llp/0015-query-and-datasets.spec.md index 67ef1b29..22b7f514 100644 --- a/llp/0015-query-and-datasets.spec.md +++ b/llp/0015-query-and-datasets.spec.md @@ -75,6 +75,13 @@ partition can't satisfy the predicate the union drops `where` for it and lets the engine filter. `columns` is always forwarded: projecting an absent column reads as null, never throws. +> **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 +> rows it yields must have. A partition narrower than the union's advertised +> column list makes a star expansion slide a later output name onto its +> neighbour's value, so the union pads each row back out to the list the scan +> advertised. + ## Collect: the ad-hoc on-ramp `hypaware collect` registers an external JSONL file (or glob) the user already diff --git a/llp/0241-scan-rows-carry-advertised-columns.decision.md b/llp/0241-scan-rows-carry-advertised-columns.decision.md new file mode 100644 index 00000000..62add5c9 --- /dev/null +++ b/llp/0241-scan-rows-carry-advertised-columns.decision.md @@ -0,0 +1,136 @@ +# LLP 0241: A Scan's Rows Carry the Column List the Scan Advertised + +**Type:** Decision +**Status:** Accepted +**Systems:** Query, Cache +**Author:** Phil / Claude +**Date:** 2026-08-15 +**Related:** LLP 0015, LLP 0029, LLP 0032, LLP 0055, LLP 0098 + +> Extends [LLP 0015](./0015-query-and-datasets.spec.md) "Multi-partition +> union": that section settled which *hints* a union may forward. This one +> settles what *shape* the rows it yields must have. + +## Context + +Issue #788 reported that over a union whose partitions have drifted schemas, +`SELECT *, git_remote FROM ai_gateway_messages` came back with `gateway_id` +holding `git_remote`'s value. The query succeeded, so the caller had no +signal that a column's value came from a different column. + +It reproduces, and not only over a union. Staging an `ai_gateway_messages` +cache with one icebird partition whose schema never had `git_remote` (the +normal post-v7 state, LLP 0032) and a second that has it: + +``` +=== lone (ONE partition, no union involved) === + SELECT *, gateway_id AS trailing FROM t + {"id":1,"gateway_id":"gw-narrow","date":"2026-05-26","schema_version":"gw-narrow"} + SELECT *, 1 AS lit FROM t + THREW TypeError: asyncRow.cells[k] is not a function + +=== drifted (one partition with git_remote, one without) === + SELECT *, git_remote AS gr FROM t + {"id":1,"gateway_id":"gw-narrow","date":"2026-05-26"} + {"id":2,...,"git_remote":"git@...","schema_version":"git@..."} + SELECT *, gateway_id AS trailing FROM t + {"id":1,"gateway_id":"gw-narrow","date":"2026-05-26","git_remote":"gw-narrow"} + {"id":2,...,"git_remote":"git@...","schema_version":"gw-wide"} +``` + +Two things to note. The value lands under whichever **declared** column sits +at the star's physical width, so it is not always `gateway_id`: on the lone +partition it is `schema_version`, and on the narrow half of the drifted union +it is `git_remote`. And the same misalignment surfaces as a hard `TypeError` +when the trailing item is a literal, so this is not purely a wrong-value bug. + + + +### The mechanism, isolated + +The defect is not in icebird, not in the parquet reader, and not in the +`withSchemaColumns` wrapper's column declaration. A hand-rolled +`AsyncDataSource` with no HypAware and no icebird in the picture reproduces it +exactly: + +```js +// declares [a, b, c, d]; yields rows whose `columns` is only [a, b] +SELECT * FROM t -> [{"a":1,"b":2}] +SELECT *, b FROM t -> [{"a":1,"b":2,"c":2}] // b's value, named c +``` + +Squirreling derives a query's output column names **once**, from the scan's +advertised list (`executeScan` returns `columns: plan.hints.columns ?? +table.columns`, and `selectColumnNames` expands the star over that). It then +fills them per row by walking that **row's own** `columns` array and +advancing a shared index. A row narrower than the advertised list under-runs +the index, so every output name after the star slides onto a neighbour. + +The engine never passes a `columns` hint for a star query (measured: `SELECT +*` and `SELECT *, b` both arrive as `scan({ columns: undefined })`), so the +advertised list for any star is the source's full `columns`, and a drifted +partition's row is always short. Nothing above the source can repair this: +the output name list is already fixed and already promised to the caller in +`QueryResults.columns` before the first row is read. + +## Decision + + + +### Rows carry the advertised list + +**A scan must yield rows whose `columns` equals the column list the scan +advertises**, that is `options.columns ?? source.columns`. A column the +partition does not physically carry gets a padded cell rather than a missing +slot. This is not a new promise to the caller: it is the schema the engine +already reported. Only the row objects disagreed with it. + +Core ships `alignRowColumns` (one row) and `alignRows` (a stream) from +`hypaware/core/query` next to `unionSources`, and applies them at the two +places a HypAware row can be narrower than what its source advertises: + +- **`unionSources.scan`**, because the union advertises the union of its + partitions' columns while each partition yields only its own. This covers + the parquet-backed unions too, where `parquetDataSource` derives a row's + `columns` from `Object.keys(data[0])`. +- **`withSchemaColumns.scan`** in the AI-gateway dataset, because the wrapper + advertises the dataset's declared schema over partitions that predate part + of it (LLP 0032), and the single-partition path returns the wrapper with no + union underneath. + +A row that already matches is returned untouched, so the ordinary case (a +partition holding every declared column) pays one length check per row. + +### The padded cell reads `undefined`, and this decides nothing new + +A padded cell resolves to `undefined` and the row's `resolved` map is left +alone, so a padded column is simply absent from it. That is deliberately the +**same** value the engine already read for a declared-but-absent column on +the row path (measured in issue #778), so this decision does not touch the +`null`/`undefined` split between the `scanColumn` and row paths, and does not +change what any existing query answers for an absent column. Whether that +split should be collapsed remains an open design question, not settled here. + +## Consequences + +- `SELECT *` renders identically. A row object gains a key per declared + column it lacks, but the value is `undefined`, which `JSON.stringify` drops + exactly as it dropped the missing key. The verified rendering over the + drifted fixture is unchanged before and after: + `{"id":1,"gateway_id":"gw-narrow","date":"2026-05-26"}`. +- `Object.keys(row).length` for a star over a drifted partition now equals + the declared column count rather than the physical one. A consumer that + enumerated a result row's keys to discover which columns a partition + physically held loses that signal. It was never a sound signal: the two + halves of a drifted union answered it differently for the same query, and + `QueryResults.columns` already reported the declared list. +- `SELECT *, ` over a drifted partition stops throwing + `TypeError: asyncRow.cells[k] is not a function`. +- The duty is on the **source**, not the engine. HypAware does not own + squirreling, and an engine that fixed this by re-deriving output names per + row would have to abandon the single static `columns` a result set + promises. Aligning at the source is the smaller and locally verifiable + change. +- Pinned by `test/core/star-expansion-drifted-union.test.js`, which asserts + the occupant of each named cell with exact equality: a row of the right + shape carrying the wrong values cannot pass. diff --git a/src/core/query/index.js b/src/core/query/index.js index 405ead7e..b5361b82 100644 --- a/src/core/query/index.js +++ b/src/core/query/index.js @@ -7,5 +7,5 @@ export { executeQuerySql, QueryExecutionBudgetError } from './sql.js' export { parquetDataSource } from './parquet-source.js' export { whereToParquetFilter } from './parquet-pushdown.js' -export { unionSources, emptySource, canPushWhere, whereColumns } from './union-source.js' +export { alignRowColumns, alignRows, unionSources, emptySource, canPushWhere, whereColumns } from './union-source.js' export { normalizeScanColumn } from './scan-column.js' diff --git a/src/core/query/union-source.js b/src/core/query/union-source.js index 2db61f0c..0049fb16 100644 --- a/src/core/query/union-source.js +++ b/src/core/query/union-source.js @@ -3,9 +3,85 @@ import { normalizeScanColumn } from './scan-column.js' /** - * @import { AsyncDataSource, ExprNode } from 'squirreling/src/types.js' + * @import { AsyncCell, AsyncRow, AsyncDataSource, ExprNode } from 'squirreling/src/types.js' */ +/** + * The cell a row gets for a column its partition does not physically carry. + * `undefined` is outside `SqlPrimitive`, hence the cast: it is nonetheless + * what the row path already reads for such a column (squirreling's `asyncRow` + * builds a cell per REQUESTED key and resolves it off an object that has no + * such key), so padding introduces no new value. + */ +const absentCell = /** @type {AsyncCell} */ (/** @type {unknown} */ (() => Promise.resolve(undefined))) + +/** + * Re-key one scanned row onto the exact column list the scan advertises, + * filling any column the row lacks with an absent cell. + * + * The SQL engine derives a query's output column names ONCE, from the scan's + * advertised list (`options.columns ?? source.columns`), then walks each row's + * own `columns` to fill them positionally. A row narrower than the advertised + * list therefore slides every later output name onto the wrong value, so + * `SELECT *, git_remote` can answer with `git_remote`'s value under the name + * of whichever column happens to sit at the star's short width. Aligning here + * costs nothing on the common path (a row that already matches is returned + * untouched) and makes the row shape match the schema the engine already told + * the caller it was returning. + * + * @ref LLP 0241#alignment [implements]: rows a scan yields carry the scan's advertised column list, not the partition's physical one + * @param {AsyncRow} row + * @param {string[]} columns + * @returns {AsyncRow} + */ +export function alignRowColumns(row, columns) { + if (row.columns === columns) return row + if (row.columns.length === columns.length) { + let same = true + for (let i = 0; i < columns.length; i++) { + if (row.columns[i] !== columns[i]) { + same = false + break + } + } + if (same) return row + } + /** @type {Record} */ + const cells = {} + for (const name of columns) cells[name] = row.cells[name] ?? absentCell + // `resolved` is keyed by name and only ever read by name, so the original + // object stays correct: a padded column is simply missing from it, which is + // the same `undefined` the padded cell resolves to. + return row.resolved ? { columns, cells, resolved: row.resolved } : { columns, cells } +} + +/** + * `alignRowColumns` over a whole row stream. + * + * A scan yields many rows sharing one `columns` array (icebird and + * `parquetDataSource` both build it once per batch), so the already-aligned + * verdict is memoized by array identity: the ordinary case, where every + * partition holds every advertised column, then costs one reference compare + * per row instead of a name-by-name walk. + * + * @param {AsyncIterable} rows + * @param {string[]} columns + * @returns {AsyncGenerator} + */ +export async function* alignRows(rows, columns) { + /** @type {string[] | undefined} */ + let alignedColumns + for await (const row of rows) { + if (row.columns === alignedColumns) { + yield row + continue + } + const out = alignRowColumns(row, columns) + if (out === row) alignedColumns = row.columns + yield out + } +} + /** * Concatenate several `AsyncDataSource`s into one logical source. Columns * are unioned, `numRows` summed, and rows yielded partition-by-partition. @@ -58,6 +134,11 @@ export function unionSources(sources) { // present but references a construct we can't safely push down (a // qualified identifier, subquery, or other non-local construct). const predicateColumns = base && base.where ? whereColumns(base.where) : undefined + // What the engine will name this scan's output columns. A partition + // that physically lacks some of them must still yield rows of this + // shape, or the star expansion slides values onto neighbouring names. + // @ref LLP 0241#alignment [implements]: the union's rows carry the union's column list, whatever each partition physically holds + const scanColumns = base?.columns ?? union.columns return { appliedWhere: false, appliedLimitOffset: false, @@ -68,9 +149,7 @@ export function unionSources(sources) { subOptions = { ...base, where: undefined } } const scan = source.scan(subOptions) - for await (const row of scan.rows()) { - yield row - } + yield* alignRows(scan.rows(), scanColumns) } }, } diff --git a/test/core/star-expansion-drifted-union.test.js b/test/core/star-expansion-drifted-union.test.js new file mode 100644 index 00000000..58f545f5 --- /dev/null +++ b/test/core/star-expansion-drifted-union.test.js @@ -0,0 +1,244 @@ +// @ts-check + +// Regression pins for LLP 0241: a scan must yield rows carrying the column +// list the scan advertises, so a star expansion cannot slide a later output +// name onto a neighbouring column's value. +// +// Every assertion here was written from a recorded run, not from reading the +// engine. On `origin/master` the shape below answered +// `SELECT *, gateway_id AS trailing` with the gateway_id value stored under +// the name `schema_version` (lone partition) or `git_remote` (the narrow half +// of a drifted union): a silently wrong answer, no error. The cell values are +// asserted by name with exact equality, because a row with the right SHAPE +// and the wrong occupant is exactly the failure being pinned. + +import test from 'node:test' +import assert from 'node:assert/strict' +import fs from 'node:fs/promises' +import os from 'node:os' +import path from 'node:path' + +import { collect, executeSql } from 'squirreling' + +import { appendRowsToSourceTable } from '../../src/core/cache/partition.js' +import { createQueryStorageService } from '../../src/core/cache/storage.js' +import { alignRowColumns, unionSources } from '../../src/core/query/union-source.js' +import { + createDataSource, + DATASET_NAME, + discoverParts, +} from '../../hypaware-core/plugins-workspace/ai-gateway/src/dataset.js' + +/** + * @import { ColumnSpec, QueryScope } from '../../hypaware-plugin-kernel-types.js' + * @import { AsyncDataSource, AsyncRow, SqlPrimitive } from 'squirreling/src/types.js' + */ + +/** Columns every fixture partition carries. */ +/** @type {ColumnSpec[]} */ +const NARROW_COLUMNS = [ + { name: 'id', type: 'INT32', nullable: false }, + { name: 'gateway_id', type: 'STRING', nullable: true }, + { name: 'date', type: 'STRING', nullable: false }, +] + +/** The same plus `git_remote`, a real v7 capture column (LLP 0032). */ +/** @type {ColumnSpec[]} */ +const WIDE_COLUMNS = [...NARROW_COLUMNS, { name: 'git_remote', type: 'STRING', nullable: true }] + +const REMOTE = 'git@example.com:acme/app.git' + +/** + * Stage an icebird-backed `ai_gateway_messages` cache and return the source a + * query runs against. + * + * - `lone`: ONE partition whose iceberg schema never had `git_remote`, so + * `createDataSource` skips `unionSources` and only `withSchemaColumns` + * stands between the query and icebird. + * - `drifted`: TWO partitions, one with `git_remote` and one without. + * + * @param {'lone' | 'drifted'} shape + * @returns {Promise<{ cacheRoot: string, source: AsyncDataSource }>} + */ +async function stageFixture(shape) { + const cacheRoot = await fs.mkdtemp(path.join(os.tmpdir(), `hyp-star-${shape}-`)) + await appendRowsToSourceTable( + cacheRoot, DATASET_NAME, ['source=claude'], + NARROW_COLUMNS, [{ id: 1, gateway_id: 'gw-narrow', date: '2026-05-26' }] + ) + if (shape === 'drifted') { + await appendRowsToSourceTable( + cacheRoot, DATASET_NAME, ['source=codex'], + WIDE_COLUMNS, [{ id: 2, gateway_id: 'gw-wide', date: '2026-05-27', git_remote: REMOTE }] + ) + } + const storage = createQueryStorageService({ cacheRoot }) + /** @type {QueryScope} */ + const scope = { limit: 1000 } + const partitions = await discoverParts({ cacheDir: cacheRoot, scope, config: { version: 2 } }) + const source = await createDataSource(partitions, { scope, storage }) + return { cacheRoot, source } +} + +/** + * @param {'lone' | 'drifted'} shape + * @param {(source: AsyncDataSource) => Promise} body + */ +async function withFixture(shape, body) { + const { cacheRoot, source } = await stageFixture(shape) + try { + await body(source) + } finally { + await fs.rm(cacheRoot, { recursive: true, force: true }) + } +} + +/** + * Run a SELECT the way `hyp query sql` does, ordered by `id` so partition + * scan order cannot make an assertion flap. + * + * @param {AsyncDataSource} source + * @param {string} query + * @returns {Promise[]>} + */ +async function runSql(source, query) { + const rows = await collect(executeSql({ tables: { t: source }, query })) + return /** @type {Record[]} */ (rows) +} + +// --- the reported defect, at the SQL surface --- + +test('star expansion: a trailing column keeps its own name over a lone drifted partition', async () => { + await withFixture('lone', async (source) => { + const rows = await runSql(source, 'SELECT *, gateway_id AS trailing FROM t ORDER BY id') + assert.equal(rows.length, 1) + assert.strictEqual(rows[0].trailing, 'gw-narrow', 'the aliased column holds its own value') + assert.strictEqual(rows[0].gateway_id, 'gw-narrow', 'the star copy is unchanged') + // Before the fix this held 'gw-narrow': the trailing column's value was + // written under the name of the declared column sitting at the star's + // (short) physical width. + assert.strictEqual(rows[0].schema_version, undefined, 'no value slid into the neighbouring declared column') + }) +}) + +test('star expansion: a trailing column keeps its own name on both halves of a drifted union', async () => { + await withFixture('drifted', async (source) => { + const rows = await runSql(source, 'SELECT *, gateway_id AS trailing FROM t ORDER BY id') + assert.equal(rows.length, 2) + assert.strictEqual(rows[0].id, 1) + assert.strictEqual(rows[0].trailing, 'gw-narrow') + // The narrow partition's short row put 'gw-narrow' here before the fix. + assert.strictEqual(rows[0].git_remote, undefined, 'the column the narrow partition lacks stays absent') + assert.strictEqual(rows[0].schema_version, undefined) + assert.strictEqual(rows[1].id, 2) + assert.strictEqual(rows[1].trailing, 'gw-wide') + assert.strictEqual(rows[1].git_remote, REMOTE, 'the wide partition still reads its own git_remote') + // The wide partition's row was one column short of the declared list, so + // 'gw-wide' landed here before the fix. + assert.strictEqual(rows[1].schema_version, undefined) + }) +}) + +test('star expansion: SELECT *, git_remote reads git_remote, not a neighbour', async () => { + // The exact shape reported in issue #788. + await withFixture('drifted', async (source) => { + const rows = await runSql(source, 'SELECT *, git_remote AS gr FROM t ORDER BY id') + assert.equal(rows.length, 2) + assert.strictEqual(rows[0].gr, undefined, 'the partition that predates the column reads no value') + assert.strictEqual(rows[1].gr, REMOTE) + // Before the fix the remote URL came back under `schema_version`. + assert.strictEqual(rows[1].schema_version, undefined, 'the remote did not land in a neighbouring column') + assert.strictEqual(rows[1].gateway_id, 'gw-wide', 'no declared column holds another column\'s value') + }) +}) + +test('star expansion: a literal beside a star does not crash over a drifted partition', async () => { + // On master this threw `TypeError: asyncRow.cells[k] is not a function`, + // the same misalignment surfacing as a crash instead of a wrong value. + await withFixture('lone', async (source) => { + const rows = await runSql(source, 'SELECT *, 1 AS lit FROM t') + assert.equal(rows.length, 1) + assert.strictEqual(rows[0].lit, 1) + assert.strictEqual(rows[0].id, 1) + }) +}) + +test('star expansion: a bare star still renders only the columns a partition holds', async () => { + // Padding must not change what a plain `SELECT *` shows: an absent column + // resolves to undefined, which JSON drops exactly as it did before. + await withFixture('drifted', async (source) => { + const rows = await runSql(source, 'SELECT * FROM t ORDER BY id') + assert.equal(JSON.stringify(rows[0]), '{"id":1,"gateway_id":"gw-narrow","date":"2026-05-26"}') + assert.equal( + JSON.stringify(rows[1]), + `{"id":2,"gateway_id":"gw-wide","date":"2026-05-27","git_remote":${JSON.stringify(REMOTE)}}` + ) + }) +}) + +// --- the same defect at the core union, independent of icebird --- + +/** + * A minimal source that declares `columns` but yields rows carrying only the + * keys each object actually has, which is what a drifted partition does. + * + * @param {string[]} columns + * @param {Record[]} objects + * @returns {AsyncDataSource} + */ +function narrowSource(columns, objects) { + return { + columns, + numRows: objects.length, + scan() { + return { + appliedWhere: false, + appliedLimitOffset: false, + async *rows() { + for (const obj of objects) { + const keys = Object.keys(obj) + /** @type {Record Promise>} */ + const cells = {} + for (const key of keys) cells[key] = () => Promise.resolve(obj[key]) + yield { columns: keys, cells, resolved: obj } + } + }, + } + }, + } +} + +test('union: a partition missing a unioned column still yields the union column list', async () => { + const union = unionSources([ + narrowSource(['a', 'b'], [{ a: 1, b: 2 }]), + narrowSource(['a', 'b', 'c'], [{ a: 3, b: 4, c: 5 }]), + ]) + assert.deepEqual(union.columns, ['a', 'b', 'c']) + const rows = await runSql(union, 'SELECT *, b AS trailing FROM t ORDER BY a') + assert.equal(rows.length, 2) + // Before the fix the first row's `b` value (2) came back as `c`. + assert.strictEqual(rows[0].trailing, 2) + assert.strictEqual(rows[0].c, undefined, 'the column the first partition lacks stays absent') + assert.strictEqual(rows[1].trailing, 4) + assert.strictEqual(rows[1].c, 5) +}) + +// --- the alignment helper itself --- + +test('alignRowColumns: pads a short row and leaves an already-aligned row untouched', async () => { + /** @type {AsyncRow} */ + const short = { + columns: ['a', 'b'], + cells: { a: () => Promise.resolve(1), b: () => Promise.resolve(2) }, + resolved: { a: 1, b: 2 }, + } + const padded = alignRowColumns(short, ['a', 'b', 'c']) + assert.deepEqual(padded.columns, ['a', 'b', 'c']) + assert.strictEqual(await padded.cells.a(), 1) + assert.strictEqual(await padded.cells.b(), 2) + assert.strictEqual(await padded.cells.c(), undefined, 'the padded cell exists and reads undefined') + assert.deepEqual(padded.resolved, { a: 1, b: 2 }, 'resolved keeps only the values the row really had') + + const aligned = alignRowColumns(short, ['a', 'b']) + assert.strictEqual(aligned, short, 'an already-aligned row is returned as-is, not rebuilt') +}) From 2cd23074e29f1e65727bfe93d0ecc660ee45a24d Mon Sep 17 00:00:00 2001 From: test Date: Sat, 15 Aug 2026 10:12:44 +0000 Subject: [PATCH 2/3] Record the WHERE/ORDER BY change padding also makes (#788) Review measured a second observable delta the PR did not claim: on the drifted two-partition fixture, `SELECT * FROM t WHERE git_remote IS NULL`, `WHERE git_remote = 'zzz'` and `ORDER BY git_remote` all threw `ColumnNotFoundError` on master and now answer. A clause the engine evaluates above the scan reads the column off `row.cells`, which missed on a short row. That is the behaviour LLP 0015 already required of a union, so it is a widening of the fix rather than a regression, but "the one observable difference is Object.keys(row).length" was not true as written. LLP 0241 now records it, along with the measured cost of the per-row rebuild (~25ms to ~150ms for a star over 20k rows of a 3-of-57 partition; unchanged over a partition holding every declared column), and the test file pins the three queries. The new test fails with the two call sites reverted. Co-Authored-By: Claude --- ...-rows-carry-advertised-columns.decision.md | 34 +++++++++++++++++-- .../core/star-expansion-drifted-union.test.js | 24 +++++++++++++ 2 files changed, 55 insertions(+), 3 deletions(-) diff --git a/llp/0241-scan-rows-carry-advertised-columns.decision.md b/llp/0241-scan-rows-carry-advertised-columns.decision.md index 62add5c9..6004718f 100644 --- a/llp/0241-scan-rows-carry-advertised-columns.decision.md +++ b/llp/0241-scan-rows-carry-advertised-columns.decision.md @@ -107,9 +107,19 @@ A padded cell resolves to `undefined` and the row's `resolved` map is left alone, so a padded column is simply absent from it. That is deliberately the **same** value the engine already read for a declared-but-absent column on the row path (measured in issue #778), so this decision does not touch the -`null`/`undefined` split between the `scanColumn` and row paths, and does not -change what any existing query answers for an absent column. Whether that -split should be collapsed remains an open design question, not settled here. +`null`/`undefined` split between the `scanColumn` and row paths, and no query +that already returned a value returns a different one. Whether that split +should be collapsed remains an open design question, not settled here. + +It does change queries that did not return a value at all. A clause the +engine evaluates above the scan (a `WHERE` a partition could not accept, an +`ORDER BY`) reads the absent column off `row.cells`, and on a short row that +lookup missed and raised `ColumnNotFoundError`. On a padded row it reads +`undefined`, so those queries now answer instead of throwing. That is the +behaviour [LLP 0015](./0015-query-and-datasets.spec.md) 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. It is recorded below +rather than left implicit. ## Consequences @@ -126,6 +136,24 @@ split should be collapsed remains an open design question, not settled here. `QueryResults.columns` already reported the declared list. - `SELECT *, ` over a drifted partition stops throwing `TypeError: asyncRow.cells[k] is not a function`. +- A query whose `WHERE` or `ORDER BY` names a column some partition lacks + stops throwing `ColumnNotFoundError` and answers. Measured on the drifted + two-partition fixture, before to after: + `SELECT * FROM t WHERE git_remote IS NULL` threw, now returns the narrow + row; `WHERE git_remote = 'zzz'` threw, now returns no rows; `ORDER BY + git_remote` (either direction) threw, now returns both rows. So the fix is + wider than the star expansion that motivated it: `Object.keys(row).length` + is the only change to a query that already succeeded, not the only change + overall. +- The padding is a per-row rebuild, and it is not free on a drifted + partition. Measured over 20k rows of a 3-of-57 partition, `SELECT *` went + from ~25ms to ~150ms; over a partition holding every declared column it is + unchanged (~550ms both ways), because such rows match by content and the + per-stream memo then costs one reference compare. The multi-partition + AI-gateway path rebuilds a narrow row twice, once to the union's physical + column list and again to the wrapper's declared list. Accepted: the cost + buys row objects that agree with the schema the engine already promised, + and it scales with the declared width the caller asked for. - The duty is on the **source**, not the engine. HypAware does not own squirreling, and an engine that fixed this by re-deriving output names per row would have to abandon the single static `columns` a result set diff --git a/test/core/star-expansion-drifted-union.test.js b/test/core/star-expansion-drifted-union.test.js index 58f545f5..b4cdc1d0 100644 --- a/test/core/star-expansion-drifted-union.test.js +++ b/test/core/star-expansion-drifted-union.test.js @@ -176,6 +176,30 @@ test('star expansion: a bare star still renders only the columns a partition hol }) }) +test('a clause above the scan reads a column some partition lacks without throwing', async () => { + // Padding is wider than the star expansion that motivated it. A `WHERE` the + // union could not push down, or an `ORDER BY`, is evaluated above the scan + // and reads the column off `row.cells`; on master that lookup missed on the + // narrow partition's short row and raised + // `ColumnNotFoundError: Column "git_remote" not found. Available columns: + // id, gateway_id, date (row 1)`. LLP 0015 already required that a union + // never throws here, so this pins the recorded before/after. + // The star matters: only a star scan carries no `columns` hint, so only a + // star leaves the partition free to yield a row narrower than the clause + // needs. `SELECT id FROM t WHERE git_remote IS NULL` hints both columns and + // never reproduced this. + await withFixture('drifted', async (source) => { + const isNull = await runSql(source, 'SELECT * FROM t WHERE git_remote IS NULL ORDER BY id') + assert.deepEqual(isNull.map((r) => r.id), [1], 'only the partition lacking the column matches IS NULL') + + const noMatch = await runSql(source, "SELECT * FROM t WHERE git_remote = 'zzz' ORDER BY id") + assert.deepEqual(noMatch, [], 'a predicate no row satisfies answers empty, it does not throw') + + const ordered = await runSql(source, 'SELECT * FROM t ORDER BY git_remote') + assert.deepEqual(ordered.map((r) => r.id).sort(), [1, 2], 'ordering by the drifted column keeps every row') + }) +}) + // --- the same defect at the core union, independent of icebird --- /** From a407168a7e36d4ff85727e4cf8255c4f93ff3c61 Mon Sep 17 00:00:00 2001 From: test Date: Sat, 15 Aug 2026 11:05:20 +0000 Subject: [PATCH 3/3] Settle the double-alignment question and record the key-order change (#788) Round 2 measured two things round 1 left open. Neither alignment pass is removable. The wrapper cannot defer to the union: the union aligns to what its partitions PHYSICALLY hold, the wrapper to the DECLARED schema, a strict superset whenever a column is absent from every partition. Removing only the wrapper's pass puts the reported defect straight back on the multi-partition path, including the `SELECT *, ` crash. The union cannot defer to the wrapper either, since `unionSources` is a core export with no wrapper above it in otel, gascity, s3 and context-graph, and dropping it measured ~178ms against ~173ms with it, inside the noise: the union rebuild spans a handful of columns, the wrapper's spans all 57. A star over a partition whose physical column order differs from the advertised list now renders its keys in the advertised order. Measured at the core union over `[a, b]` and `[b, a]`: `{"b":4,"a":3}` before, `{"a":3,"b":4}` now, same values. That is a third change to a query that already succeeded, so "Object.keys(row).length is the only one" was still not exact. The new order is the one QueryResults.columns already reported. Pinned. Also recorded that the widened WHERE/ORDER BY answers were checked against the hinted form of each query, which never went short and never threw. Co-Authored-By: Claude --- ...-rows-carry-advertised-columns.decision.md | 35 +++++++++++++++---- .../core/star-expansion-drifted-union.test.js | 15 ++++++++ 2 files changed, 44 insertions(+), 6 deletions(-) diff --git a/llp/0241-scan-rows-carry-advertised-columns.decision.md b/llp/0241-scan-rows-carry-advertised-columns.decision.md index 6004718f..775f208c 100644 --- a/llp/0241-scan-rows-carry-advertised-columns.decision.md +++ b/llp/0241-scan-rows-carry-advertised-columns.decision.md @@ -142,18 +142,41 @@ rather than left implicit. `SELECT * FROM t WHERE git_remote IS NULL` threw, now returns the narrow row; `WHERE git_remote = 'zzz'` threw, now returns no rows; `ORDER BY git_remote` (either direction) threw, now returns both rows. So the fix is - wider than the star expansion that motivated it: `Object.keys(row).length` - is the only change to a query that already succeeded, not the only change - overall. + wider than the star expansion that motivated it, and the key-count growth + above is not the only change overall. The answers are the ones the hinted + form of each query already gave on both trees (`SELECT id FROM t WHERE + git_remote IS NULL` and friends carry a `columns` hint, so they never went + short and never threw), which is the reference these were checked against. +- A star over a partition whose physical column order differs from the + advertised list now renders its keys in the advertised order. Measured at + the core union over partitions declaring `[a, b]` and `[b, a]`, `SELECT *` + returned `{"b":4,"a":3}` for the second partition before and `{"a":3,"b":4}` + now. The values are unchanged, and the new order is the one + `QueryResults.columns` already reported, so this settles a disagreement + rather than creating one. Carrying the advertised list means carrying its + order, not just its membership. - The padding is a per-row rebuild, and it is not free on a drifted partition. Measured over 20k rows of a 3-of-57 partition, `SELECT *` went from ~25ms to ~150ms; over a partition holding every declared column it is unchanged (~550ms both ways), because such rows match by content and the per-stream memo then costs one reference compare. The multi-partition AI-gateway path rebuilds a narrow row twice, once to the union's physical - column list and again to the wrapper's declared list. Accepted: the cost - buys row objects that agree with the schema the engine already promised, - and it scales with the declared width the caller asked for. + column list and again to the wrapper's declared list, and **neither pass is + removable**. The wrapper cannot defer to the union: the union aligns to the + union of what its partitions **physically** hold, while the wrapper + advertises the **declared** schema, a strict superset whenever a column is + absent from every partition (the normal post-bump state, LLP 0032). + Removing only the wrapper's pass puts the reported defect straight back on + the multi-partition path, `SELECT *, ` crash included. The union + cannot defer to the wrapper either: `unionSources` is a core export with no + wrapper above it in otel, gascity, s3 and the context-graph datasets, so + skipping there would need a flag threaded down from the wrapper. And that + coupling would buy nothing measurable: the union rebuild spans the handful + of columns a partition physically holds while the wrapper rebuild spans all + 57, so dropping the union pass measured ~178ms against ~173ms with it, + inside the run-to-run noise. Accepted: the cost buys row objects that agree + with the schema the engine already promised, and it scales with the + declared width the caller asked for. - The duty is on the **source**, not the engine. HypAware does not own squirreling, and an engine that fixed this by re-deriving output names per row would have to abandon the single static `columns` a result set diff --git a/test/core/star-expansion-drifted-union.test.js b/test/core/star-expansion-drifted-union.test.js index b4cdc1d0..faf1bedc 100644 --- a/test/core/star-expansion-drifted-union.test.js +++ b/test/core/star-expansion-drifted-union.test.js @@ -247,6 +247,21 @@ test('union: a partition missing a unioned column still yields the union column assert.strictEqual(rows[1].c, 5) }) +test('union: a star renders keys in the advertised order, not each partition physical order', async () => { + // Carrying the advertised list means carrying its ORDER, not just its + // membership. Recorded run: before the fix the second partition's row came + // back as {"b":4,"a":3}, which disagreed with the ["a","b"] that + // QueryResults.columns had already reported for the same query. + const union = unionSources([ + narrowSource(['a', 'b'], [{ a: 1, b: 2 }]), + narrowSource(['b', 'a'], [{ b: 4, a: 3 }]), + ]) + assert.deepEqual(union.columns, ['a', 'b']) + const rows = await runSql(union, 'SELECT * FROM t ORDER BY a') + assert.deepEqual(rows.map((r) => Object.keys(r)), [['a', 'b'], ['a', 'b']]) + assert.deepEqual(rows, [{ a: 1, b: 2 }, { a: 3, b: 4 }], 'reordering keys moved no value') +}) + // --- the alignment helper itself --- test('alignRowColumns: pads a short row and leaves an already-aligned row untouched', async () => {