From be9917c0b8583899b05ad64494163e8c86660d43 Mon Sep 17 00:00:00 2001 From: Phillip Cunliffe Date: Wed, 12 Aug 2026 14:43:47 -0700 Subject: [PATCH 1/3] Delete our duplicate WHERE-pushdown converter, use icebird's (LLP 0212) `src/core/query/parquet-pushdown.js` and `icebird/src/sql/whereFilter.js` were two ports of the same Hyperparam original. icebird's kept moving and ours did not, and the drift cost query time and correctness. Typed literals never converted. squirreling parses `TIMESTAMP '2026-08-11T00:00:00Z'` as a `cast` wrapping a string literal; icebird constant-folds that, ours required a bare `literal` operand and returned undefined for the whole predicate, because AND is all-or-nothing. Every timestamp-bounded query therefore pushed nothing down to the cache tier. Measured on the production central server, org hyperparam: one grouped sessions-list scan took 11.4s bounded on `message_created_at` against 7.3s bounded on `date`, same rows, same projection. Any cast unwrapped at boolean position. `WHERE CAST(a = 1 AS TEXT)` pushed down as `a = 1`, but the engine evaluates that cast to the string 'false', which is truthy, so the pushdown dropped rows the query selects, and a converted filter sets appliedWhere so the engine does not re-filter to catch it. icebird gates the unwrap to truthiness-preserving casts. `coerceBigInt` is dropped rather than ported. `filterStrict: false` compares through `==`, so `5n == 5` either way, while hyparquet's bloom hashing rejects a bigint for INT32/FLOAT/DOUBLE: the coercion bought nothing on INT64 and disabled bloom pruning everywhere else. That relies on hyparquet >= 1.28.1, where `$in`/`$nin` match through `equals()` rather than `Array.prototype.includes`; this repo pins 1.28.1. A companion bump of hypaware-server's own 1.27.1 pin follows separately. icebird is already a direct dependency and deep `icebird/src/*.js` imports are the established pattern here, so this costs no new dependency and no type fidelity. The public `hypaware/core/query` surface is unchanged. Tests assert plain numbers where they asserted bigints, and cover the two shapes the drift produced: the folded TIMESTAMP literal (both as a unit and end to end through a real parquet scan, since a mis-folded literal would drop rows rather than merely lose pruning) and the truthiness-cast guard. Co-Authored-By: Claude Opus 5 (1M context) --- llp/0212-one-pushdown-converter.decision.md | 102 +++++++++ src/core/query/parquet-pushdown.js | 221 +++----------------- test/core/parquet-source.test.js | 101 +++++++-- 3 files changed, 224 insertions(+), 200 deletions(-) create mode 100644 llp/0212-one-pushdown-converter.decision.md diff --git a/llp/0212-one-pushdown-converter.decision.md b/llp/0212-one-pushdown-converter.decision.md new file mode 100644 index 00000000..758708f6 --- /dev/null +++ b/llp/0212-one-pushdown-converter.decision.md @@ -0,0 +1,102 @@ +# LLP 0212: One WHERE-to-parquet-filter converter, owned by icebird + +**Type:** Decision +**Status:** Accepted +**Systems:** Query, Cache +**Author:** Phil / Claude +**Date:** 2026-08-12 +**Related:** LLP 0098 (pushed the predicate down through `scanColumn`; this one settles *whose* converter does the pushing), LLP 0015 + +> The kernel keeps no WHERE-to-`ParquetQueryFilter` converter of its own. +> `src/core/query/parquet-pushdown.js` re-exports icebird's +> `whereToParquetFilter`. The cache tier and the archive tier now convert +> predicates identically, because they run the same function. + +## Context {#context} + +Two converters existed. `src/core/query/parquet-pushdown.js` (the cache tier, +via `parquet-source.js`) and `icebird/src/sql/whereFilter.js` (the archive +tier, via `icebergDataSource`) both began as ports of the Hyperparam app's +`lib/tools/parquetPushdownFilter.ts`. Same function names, same structure, +same De Morgan comments. icebird's copy kept moving; ours did not. + +The drift was invisible until it was measured. hypscope's sessions surface +bounds its day windows on `message_created_at` with typed literals +(`TIMESTAMP '2026-08-11T00:00:00Z'`), which squirreling parses as a `cast` +node wrapping a string literal. icebird constant-folds that shape +(`staticLiteral` / `foldCast`). Our copy required a bare `literal` operand, +so `extractColumnAndValue` returned nothing, and because AND is +all-or-nothing (`if (!left || !right) return undefined`) the *entire* +predicate converted to `undefined`. Every timestamp-bounded query pushed +nothing down to the cache tier. + +Measured on the production central server, org `hyperparam`, 2026-08-12: one +grouped sessions-list scan, identical projection and rows, took **11.4s** +bounded on `message_created_at` against **7.3s** bounded on `date`. The +685 MB / 752-file cache tier was read without row-group pruning in the first +case and with it in the second. + +The same audit found a second divergence, this one a correctness bug rather +than a cost. Our `convertExpr` unwrapped **any** cast at boolean position: + +```js +if (node.type === 'cast') return convertExpr(node.expr, negate) +``` + +So `WHERE CAST(a = 1 AS TEXT)` pushed down as `a = 1`. The engine evaluates +that cast to the string `'false'`, which is truthy, so the pushdown dropped +rows the query selects, and because a converted filter sets `appliedWhere` +the engine does not re-filter to catch it. icebird gates the unwrap to casts +that preserve truthiness (boolean and numeric targets) and falls back to the +engine otherwise. + +## Decision {#decision} + +**icebird owns the converter.** `parquet-pushdown.js` becomes a re-export of +`whereToParquetFilter` from `icebird/src/sql/whereFilter.js`; the public +`hypaware/core/query` surface is unchanged, so no consumer moves. + +icebird is already a direct dependency, and deep-importing `icebird/src/*.js` +is the established pattern in this repo (`src/core/cache/retention.js`, +`src/core/cache/iceberg/stream_append.js`, a dozen sites). icebird's exports +map publishes `./src/*.js` with matching `types/`, so the swap costs no +type fidelity. + +### `coerceBigInt` is dropped, not ported {#no-bigint-coercion} + +The one thing our copy had that icebird's lacks was `coerceBigInt`, which +turned every integer literal into a `bigint` so it would compare equal to a +bigint-decoded INT64 column. Nothing needed it, and it was costing us: + +- `filterStrict: false` (what `parquet-source.js` and icebird both pass) + compares through `equals()`, which falls back to `==`, and `5n == 5`. The + relational operators compare mixed bigint/number natively. +- hyparquet's bloom hashing (`hashParquetValue`) **rejects** a bigint for + INT32, FLOAT and DOUBLE columns and returns `undefined`, which disables + bloom pruning. The coercion was buying nothing on INT64 and silently + turning off a pruning path on every other numeric column. + +### Floor: hyparquet >= 1.28.1 {#hyparquet-floor} + +`$in` / `$nin` are the one place the bigint/number distinction is load +bearing. hyparquet 1.27.x matched them with `Array.prototype.includes`, which +is SameValueZero: a number-valued `$in` against an INT64 column matches no +rows. 1.28.1 routes them through `matchesIn` -> `equals(value, target, +strict)`, which handles the mixed case. This repo pins 1.28.1. A consumer +that forces the kernel onto 1.27.x would get wrong results, not slow ones. + +## Consequences {#consequences} + +- Timestamp-bounded predicates prune the cache tier. Worth ~4s of the ~9s + sessions-list batch-0 scan measured above; the rest of that scan is a + separate matter (there is no partition-level pruning: `sql.js` calls + `discoverPartitions` with no WHERE, so every raw query still opens all 752 + cache files before any filter runs). +- The truthiness-cast pushdown bug is gone. +- Bloom pruning is restored for INT32/FLOAT/DOUBLE equality. +- 188 lines of converter deleted against 33 of re-export and rationale. Future + converter fixes land once, in icebird, and both tiers get them. +- Tests now assert plain numbers where they asserted bigints, and cover the + two shapes the drift produced: a folded `TIMESTAMP` literal (unit and + end-to-end through a real parquet scan, since a mis-folded literal would + drop rows rather than merely lose pruning) and the truthiness-cast guard. diff --git a/src/core/query/parquet-pushdown.js b/src/core/query/parquet-pushdown.js index 89a364cd..17d0cb7f 100644 --- a/src/core/query/parquet-pushdown.js +++ b/src/core/query/parquet-pushdown.js @@ -1,190 +1,35 @@ // @ts-check -/** - * Convert a squirreling `WHERE` clause AST into a hyparquet - * `ParquetQueryFilter` (a MongoDB-style predicate) so the scan can push - * the predicate down to the parquet reader. Returns `undefined` whenever - * the expression cannot be fully and faithfully converted. The caller - * must then leave `appliedWhere` false and let the SQL engine filter the - * rows itself. - * - * Ported from the Hyperparam app (`lib/tools/parquetPushdownFilter.ts`), - * which drives the same squirreling + hyparquet stack. The node-type - * discriminants match `squirreling@0.12` (`unary`, `binary`, - * `in valuelist`, `cast`, `identifier`, `literal`). - * - * @import { BinaryNode, BinaryOp, ComparisonOp, ExprNode, InValuesNode, SqlPrimitive } from 'squirreling/src/types.js' - * @import { ParquetQueryFilter } from 'hyparquet' - */ - -/** - * @param {ExprNode | undefined} where - * @returns {ParquetQueryFilter | undefined} - */ -export function whereToParquetFilter(where) { - if (!where) return undefined - return convertExpr(where, false) -} - -/** - * @param {ExprNode} node - * @param {boolean} negate - * @returns {ParquetQueryFilter | undefined} - */ -function convertExpr(node, negate) { - if (node.type === 'unary' && node.op === 'NOT') { - return convertExpr(node.argument, !negate) - } - if (node.type === 'unary' && (node.op === 'IS NULL' || node.op === 'IS NOT NULL')) { - if (node.argument.type !== 'identifier') return undefined - const isNull = (node.op === 'IS NULL') !== negate - return { [node.argument.name]: { [isNull ? '$eq' : '$ne']: null } } - } - if (node.type === 'binary') { - return convertBinary(node, negate) - } - if (node.type === 'in valuelist') { - return convertInValues(node, negate) - } - if (node.type === 'cast') { - return convertExpr(node.expr, negate) - } - // Non-convertible node types (functions, subqueries, CASE, …) fall - // through to undefined so the engine applies the predicate itself. - return undefined -} - -/** - * @param {BinaryNode} node - * @param {boolean} negate - * @returns {ParquetQueryFilter | undefined} - */ -function convertBinary(node, negate) { - const { op, left, right } = node - if (op === 'AND') { - const leftFilter = convertExpr(left, negate) - const rightFilter = convertExpr(right, negate) - if (!leftFilter || !rightFilter) return undefined - // De Morgan: NOT (a AND b) === (NOT a) OR (NOT b) - return negate ? { $or: [leftFilter, rightFilter] } : { $and: [leftFilter, rightFilter] } - } - if (op === 'OR') { - // `$nor` already expresses NOT(a OR b), so the children are converted - // un-negated and the wrapper carries the negation, propagating - // `negate` into them as well would double-negate. - const leftFilter = convertExpr(left, false) - const rightFilter = convertExpr(right, false) - if (!leftFilter || !rightFilter) return undefined - return negate ? { $nor: [leftFilter, rightFilter] } : { $or: [leftFilter, rightFilter] } - } - // LIKE has no parquet-filter equivalent; let the engine handle it. - if (op === 'LIKE') return undefined - - const { column, value, flipped } = extractColumnAndValue(left, right) - if (column === undefined || value === undefined) return undefined - - const mongoOp = mapOperator(op, flipped, negate) - if (!mongoOp) return undefined - return { [column]: { [mongoOp]: value } } -} - -/** - * Pull a `column op literal` (or `literal op column`) shape out of a - * binary node's operands. Returns `flipped: true` when the literal was - * on the left so the caller can mirror the comparison operator. - * - * @param {ExprNode} left - * @param {ExprNode} right - * @returns {{ column: string | undefined, value: SqlPrimitive | undefined, flipped: boolean }} - */ -function extractColumnAndValue(left, right) { - if (left.type === 'identifier' && right.type === 'literal') { - return { column: left.name, value: coerceBigInt(right.value), flipped: false } - } - if (left.type === 'literal' && right.type === 'identifier') { - return { column: right.name, value: coerceBigInt(left.value), flipped: true } - } - return { column: undefined, value: undefined, flipped: false } -} - -/** - * @param {BinaryOp} op - * @param {boolean} flipped - * @param {boolean} negate - * @returns {'$lt' | '$lte' | '$gt' | '$gte' | '$eq' | '$ne' | undefined} - */ -function mapOperator(op, flipped, negate) { - if (!isComparisonOp(op)) return undefined - let mapped = op - if (negate) mapped = neg(mapped) - if (flipped) mapped = flip(mapped) - if (mapped === '<') return '$lt' - if (mapped === '<=') return '$lte' - if (mapped === '>') return '$gt' - if (mapped === '>=') return '$gte' - if (mapped === '=' || mapped === '==') return '$eq' - return '$ne' -} - -/** - * @param {ComparisonOp} op - * @returns {ComparisonOp} - */ -function neg(op) { - if (op === '<') return '>=' - if (op === '<=') return '>' - if (op === '>') return '<=' - if (op === '>=') return '<' - if (op === '=' || op === '==') return '!=' - // negation of `!=` / `<>` is equality - return '=' -} - -/** - * @param {ComparisonOp} op - * @returns {ComparisonOp} - */ -function flip(op) { - if (op === '<') return '>' - if (op === '<=') return '>=' - if (op === '>') return '<' - if (op === '>=') return '<=' - return op -} - -/** - * @param {string} op - * @returns {op is ComparisonOp} - */ -function isComparisonOp(op) { - return op === '=' || op === '==' || op === '!=' || op === '<>' || op === '<' || op === '>' || op === '<=' || op === '>=' -} - -/** - * Coerce integer literals to `bigint` so they compare equal to parquet - * INT64 columns, which hyparquet decodes as `bigint`. Non-integer and - * non-number values pass through unchanged. - * - * @param {SqlPrimitive} value - * @returns {SqlPrimitive} - */ -function coerceBigInt(value) { - if (typeof value === 'number' && Number.isInteger(value)) return BigInt(value) - return value -} - -/** - * @param {InValuesNode} node - * @param {boolean} negate - * @returns {ParquetQueryFilter | undefined} - */ -function convertInValues(node, negate) { - if (node.expr.type !== 'identifier') return undefined - /** @type {SqlPrimitive[]} */ - const values = [] - for (const val of node.values) { - if (val.type !== 'literal') return undefined - values.push(coerceBigInt(val.value)) - } - return { [node.expr.name]: { [negate ? '$nin' : '$in']: values } } -} +// icebird's converter, re-exported rather than reimplemented. This module and +// `icebird/src/sql/whereFilter.js` both began as ports of the Hyperparam app's +// `lib/tools/parquetPushdownFilter.ts`. icebird's copy kept moving and this one +// did not, and the drift cost both query time and correctness: +// +// - Typed literals never converted. squirreling parses +// `TIMESTAMP '2026-08-11T00:00:00Z'` as a `cast` node wrapping a string +// literal. icebird constant-folds it (`staticLiteral`/`foldCast`); the local +// copy required a bare `literal` operand and returned `undefined` for the +// whole predicate, because AND is all-or-nothing. Every timestamp-bounded +// query therefore pushed nothing down. Measured against the production +// sessions list: one grouped scan took 11.4s bounded on `message_created_at` +// versus 7.3s bounded on `date`, same rows, same projection. +// - Any cast unwrapped at boolean position. The local copy rewrote +// `WHERE CAST(a = 1 AS TEXT)` to `a = 1`, but the engine evaluates that cast +// to the string `'false'`, which is truthy, so the pushdown dropped rows the +// query selects. icebird gates the unwrap to the casts that preserve +// truthiness (boolean and numeric targets). +// +// Dropped along with the local copy: its `coerceBigInt`, which turned every +// integer literal into a `bigint`. Nothing needed it. `filterStrict: false` +// (which parquet-source.js and icebird both pass) compares through `==`, so +// `5n == 5` holds either way, while hyparquet's bloom hashing REJECTS a bigint +// for INT32, FLOAT and DOUBLE columns: the coercion was quietly buying back +// nothing and costing bloom pruning on every non-INT64 numeric column. +// +// Floor: hyparquet >= 1.28.1, where `$in`/`$nin` match through `equals()` +// instead of `Array.prototype.includes`. On 1.27.x a number-valued `$in` +// against an INT64 column (hyparquet decodes those as bigint) matches no rows, +// which is precisely what `coerceBigInt` used to paper over. +// +// @ref LLP 0212 [implements]: one pushdown converter for the whole stack, owned by icebird +export { whereToParquetFilter } from 'icebird/src/sql/whereFilter.js' diff --git a/test/core/parquet-source.test.js b/test/core/parquet-source.test.js index f112a9c6..ec65bc56 100644 --- a/test/core/parquet-source.test.js +++ b/test/core/parquet-source.test.js @@ -62,6 +62,32 @@ async function makeSource() { return parquetDataSource(file, metadata) } +/** @type {ColumnSpec[]} */ +const TIMESTAMP_COLUMNS = [ + { name: 'id', type: 'INT64', nullable: false }, + { name: 'at', type: 'TIMESTAMP', nullable: false }, +] + +// Two days either side of the 2026-08-11 window the day-bound tests select, so +// a bound that silently matched everything (or nothing) is visible. +const TIMESTAMP_ROWS = [ + { id: 1, at: '2026-08-10T23:59:59Z' }, + { id: 2, at: '2026-08-11T00:00:00Z' }, + { id: 3, at: '2026-08-11T23:59:59Z' }, + { id: 4, at: '2026-08-12T00:00:00Z' }, +] + +/** + * @returns {Promise} + */ +async function makeTimestampSource() { + const columnData = rowsToColumnSources(TIMESTAMP_COLUMNS, TIMESTAMP_ROWS) + const arrayBuffer = parquetWriteBuffer({ columnData, codec: 'SNAPPY', rowGroupSize: 2 }) + const file = asyncBufferFromBytes(new Uint8Array(arrayBuffer)) + const metadata = await parquetMetadataAsync(file) + return parquetDataSource(file, metadata) +} + /** * @param {string} sql * @returns {ExprNode | undefined} @@ -81,42 +107,73 @@ async function run(source, query) { // --- pushdown conversion ----------------------------------------------------- -test('whereToParquetFilter converts simple comparisons (integers coerced to bigint)', () => { - assert.deepEqual(whereToParquetFilter(whereOf('SELECT * FROM t WHERE id = 3')), { id: { $eq: 3n } }) - assert.deepEqual(whereToParquetFilter(whereOf('SELECT * FROM t WHERE id > 3')), { id: { $gt: 3n } }) - assert.deepEqual(whereToParquetFilter(whereOf('SELECT * FROM t WHERE id <= 3')), { id: { $lte: 3n } }) +// Integer literals stay plain numbers: `filterStrict: false` compares with +// `==` so they still match bigint-decoded INT64 columns, and hyparquet's bloom +// hashing rejects a bigint for INT32/FLOAT/DOUBLE (LLP 0212). +test('whereToParquetFilter converts simple comparisons', () => { + assert.deepEqual(whereToParquetFilter(whereOf('SELECT * FROM t WHERE id = 3')), { id: { $eq: 3 } }) + assert.deepEqual(whereToParquetFilter(whereOf('SELECT * FROM t WHERE id > 3')), { id: { $gt: 3 } }) + assert.deepEqual(whereToParquetFilter(whereOf('SELECT * FROM t WHERE id <= 3')), { id: { $lte: 3 } }) assert.deepEqual(whereToParquetFilter(whereOf("SELECT * FROM t WHERE name = 'bob'")), { name: { $eq: 'bob' } }) }) test('whereToParquetFilter mirrors flipped operands (literal on the left)', () => { - assert.deepEqual(whereToParquetFilter(whereOf('SELECT * FROM t WHERE 3 < id')), { id: { $gt: 3n } }) - assert.deepEqual(whereToParquetFilter(whereOf('SELECT * FROM t WHERE 3 >= id')), { id: { $lte: 3n } }) + assert.deepEqual(whereToParquetFilter(whereOf('SELECT * FROM t WHERE 3 < id')), { id: { $gt: 3 } }) + assert.deepEqual(whereToParquetFilter(whereOf('SELECT * FROM t WHERE 3 >= id')), { id: { $lte: 3 } }) }) test('whereToParquetFilter handles AND / OR / NOT', () => { assert.deepEqual( whereToParquetFilter(whereOf('SELECT * FROM t WHERE id >= 2 AND id <= 4')), - { $and: [{ id: { $gte: 2n } }, { id: { $lte: 4n } }] } + { $and: [{ id: { $gte: 2 } }, { id: { $lte: 4 } }] } ) assert.deepEqual( whereToParquetFilter(whereOf('SELECT * FROM t WHERE id = 1 OR id = 2')), - { $or: [{ id: { $eq: 1n } }, { id: { $eq: 2n } }] } + { $or: [{ id: { $eq: 1 } }, { id: { $eq: 2 } }] } ) - assert.deepEqual(whereToParquetFilter(whereOf('SELECT * FROM t WHERE NOT (id = 1)')), { id: { $ne: 1n } }) + assert.deepEqual(whereToParquetFilter(whereOf('SELECT * FROM t WHERE NOT (id = 1)')), { id: { $ne: 1 } }) // De Morgan: NOT (a OR b) -> $nor of the un-negated children assert.deepEqual( whereToParquetFilter(whereOf('SELECT * FROM t WHERE NOT (id = 1 OR id = 2)')), - { $nor: [{ id: { $eq: 1n } }, { id: { $eq: 2n } }] } + { $nor: [{ id: { $eq: 1 } }, { id: { $eq: 2 } }] } ) }) test('whereToParquetFilter handles IN / NOT IN / IS NULL', () => { - assert.deepEqual(whereToParquetFilter(whereOf('SELECT * FROM t WHERE id IN (1, 2)')), { id: { $in: [1n, 2n] } }) - assert.deepEqual(whereToParquetFilter(whereOf('SELECT * FROM t WHERE id NOT IN (1, 2)')), { id: { $nin: [1n, 2n] } }) + assert.deepEqual(whereToParquetFilter(whereOf('SELECT * FROM t WHERE id IN (1, 2)')), { id: { $in: [1, 2] } }) + assert.deepEqual(whereToParquetFilter(whereOf('SELECT * FROM t WHERE id NOT IN (1, 2)')), { id: { $nin: [1, 2] } }) assert.deepEqual(whereToParquetFilter(whereOf('SELECT * FROM t WHERE name IS NULL')), { name: { $eq: null } }) assert.deepEqual(whereToParquetFilter(whereOf('SELECT * FROM t WHERE name IS NOT NULL')), { name: { $ne: null } }) }) +// The regression that motivated LLP 0212: squirreling parses a typed literal +// as a cast over a string, and the pre-icebird converter required a bare +// literal operand, so every timestamp-bounded predicate converted to undefined +// and pruned nothing. +test('whereToParquetFilter folds typed literals (TIMESTAMP casts)', () => { + assert.deepEqual( + whereToParquetFilter(whereOf("SELECT * FROM t WHERE at >= TIMESTAMP '2026-08-11T00:00:00Z'")), + { at: { $gte: new Date('2026-08-11T00:00:00Z') } } + ) + // AND is all-or-nothing, so a day window only converts if both sides do + assert.deepEqual( + whereToParquetFilter(whereOf( + "SELECT * FROM t WHERE at >= TIMESTAMP '2026-08-11T00:00:00Z' AND at < TIMESTAMP '2026-08-12T00:00:00Z'" + )), + { $and: [{ at: { $gte: new Date('2026-08-11T00:00:00Z') } }, { at: { $lt: new Date('2026-08-12T00:00:00Z') } }] } + ) + // A cast the engine would evaluate to null must not become a filter + assert.equal(whereToParquetFilter(whereOf("SELECT * FROM t WHERE at >= TIMESTAMP 'not-a-day'")), undefined) +}) + +// Unwrapping a cast at boolean position is only sound when the cast preserves +// truthiness. CAST( AS TEXT) yields 'false', which is truthy, so pushing +// the bare comparison down would drop rows the query selects. +test('whereToParquetFilter only unwraps truthiness-preserving casts', () => { + assert.deepEqual(whereToParquetFilter(whereOf('SELECT * FROM t WHERE CAST(id = 1 AS INT)')), { id: { $eq: 1 } }) + assert.equal(whereToParquetFilter(whereOf('SELECT * FROM t WHERE CAST(id = 1 AS TEXT)')), undefined) +}) + test('whereToParquetFilter returns undefined for non-convertible predicates', () => { assert.equal(whereToParquetFilter(whereOf("SELECT * FROM t WHERE name LIKE 'a%'")), undefined) // a single non-convertible conjunct collapses the whole AND @@ -159,6 +216,26 @@ test('range WHERE (AND) returns the inclusive window', async () => { assert.deepEqual(rows.map((r) => Number(r.id)), [2, 3, 4]) }) +// A converted predicate sets appliedWhere, so the engine does NOT re-filter: +// a folded literal that compares wrongly against the decoded column would +// silently drop rows rather than merely lose pruning. This is the check that +// the TIMESTAMP fold is safe end to end, not just well-shaped. +test('timestamp day bounds filter correctly through the pushed-down scan', async () => { + const source = await makeTimestampSource() + const rows = await run( + source, + "SELECT id FROM t WHERE at >= TIMESTAMP '2026-08-11T00:00:00Z' AND at < TIMESTAMP '2026-08-12T00:00:00Z'" + ) + assert.deepEqual(rows.map((r) => Number(r.id)), [2, 3]) +}) + +test('a timestamp bound matching no rows returns none (and one matching all returns all)', async () => { + const none = await run(await makeTimestampSource(), "SELECT id FROM t WHERE at >= TIMESTAMP '2099-01-01T00:00:00Z'") + assert.deepEqual(none, []) + const all = await run(await makeTimestampSource(), "SELECT id FROM t WHERE at >= TIMESTAMP '2000-01-01T00:00:00Z'") + assert.equal(all.length, 4) +}) + test('LIKE falls back to engine filtering (not pushed down)', async () => { const source = await makeSource() const rows = await run(source, "SELECT name FROM t WHERE name LIKE 'a%'") From da7092c065d177fe33fea6c203b3f4fd9e330e29 Mon Sep 17 00:00:00 2001 From: Kenny Daniel Date: Thu, 13 Aug 2026 14:08:42 -0700 Subject: [PATCH 2/3] Adopt icebird's pushdown converter now the stack is three-valued squirreling 0.15.3 gives WHERE Kleene NULL semantics and icebird 0.8.22 pushes filters that agree with it, so the kernel converter is no longer ahead on correctness. Delete it for the re-export, bump hyparquet to 1.28.2 (bare relational bounds reject null cells there), and move the shape assertions to icebird's shapes. Row-set and appliedWhere tests are unchanged and stay green either way. --- llp/0219-one-pushdown-converter.decision.md | 170 ++++---- package.json | 6 +- src/core/query/parquet-pushdown.js | 438 +------------------- test/core/parquet-source.test.js | 138 +++--- 4 files changed, 173 insertions(+), 579 deletions(-) diff --git a/llp/0219-one-pushdown-converter.decision.md b/llp/0219-one-pushdown-converter.decision.md index 6181c2d5..d4529668 100644 --- a/llp/0219-one-pushdown-converter.decision.md +++ b/llp/0219-one-pushdown-converter.decision.md @@ -1,4 +1,4 @@ -# LLP 0219: The kernel keeps its own pushdown converter, and folds typed literals +# LLP 0219: One WHERE-to-parquet-filter converter, owned by icebird **Type:** Decision **Status:** Accepted @@ -7,97 +7,101 @@ **Date:** 2026-08-13 **Related:** LLP 0098 (pushed the predicate down through `scanColumn`; this one settles *whose* converter does the pushing), LLP 0015 -> The kernel keeps `src/core/query/parquet-pushdown.js` as a real converter -> rather than re-exporting icebird's. It adopts the one thing icebird's copy -> had and ours lacked, constant-folding of typed literals, and keeps the -> three-valued NULL semantics icebird's copy does not have. +> The kernel keeps no WHERE-to-`ParquetQueryFilter` converter of its own. +> `src/core/query/parquet-pushdown.js` re-exports icebird's +> `whereToParquetFilter`. The cache tier and the archive tier convert +> predicates identically, because they run the same function - and that +> function agrees with SQL three-valued logic, because the whole stack was +> brought up to it first. ## Context {#context} -Two converters exist. `src/core/query/parquet-pushdown.js` (the cache tier, +Two converters existed. `src/core/query/parquet-pushdown.js` (the cache tier, via `parquet-source.js`) and `icebird/src/sql/whereFilter.js` (the archive tier, via `icebergDataSource`) both began as ports of the Hyperparam app's `lib/tools/parquetPushdownFilter.ts`. Same function names, same structure, -same De Morgan comments. The two then drifted in *opposite* directions, and -the drift is the whole of this decision. - -**Where icebird went ahead.** hypscope's sessions surface bounds its day -windows on `message_created_at` with typed literals -(`TIMESTAMP '2026-08-11T00:00:00Z'`), which squirreling parses as a `cast` -node wrapping a string literal. icebird constant-folds that shape -(`staticLiteral` / `foldCast`). Our copy required a bare `literal` operand, -so `extractColumnAndValue` returned nothing, and because AND is -all-or-nothing the *entire* predicate converted to `undefined`. Every -timestamp-bounded query pushed nothing down to the cache tier. Measured on -the production central server, org `hyperparam`, 2026-08-12: one grouped -sessions-list scan, identical projection and rows, took **11.4s** bounded on -`message_created_at` against **7.3s** bounded on `date`. - -icebird also gates the boolean-position cast unwrap. Our copy unwrapped -**any** cast, so `WHERE CAST(a = 1 AS TEXT)` pushed down as `a = 1`; the -engine evaluates that cast to the string `'false'`, which is truthy, so the -pushdown dropped rows the query selects. - -**Where the kernel went ahead.** LLP 0098 lets a converted filter claim -`appliedWhere`, and the engine never re-filters a claimed predicate. A filter -that disagrees with SQL is therefore a wrong answer, not a lost optimisation. -Issues #728 and #734 found that the shared ancestor disagrees with SQL on -NULLs in four ways, all of which the kernel's copy has since fixed and -icebird's copy retains: - -1. bare relational operators leak NULL rows (hyparquet compares with raw JS - operators, which coerce NULL to `0`), -2. `$nor` for a negated `OR` is a two-valued complement, so a row that is - UNKNOWN for every disjunct matches, -3. `$nin` carries no NULL guard, and an all-NULL `IN` list is not a - never-match, -4. a comparison against a NULL literal converts to `$eq: null`, answering it - with `IS NULL` semantics. - -Measured on a 24-predicate battery over a nullable column, evaluated through -hyparquet's own `matchFilter` and compared against SQL three-valued truth: -the kernel's converter is wrong on **0**, `icebird@0.8.21` on **11**. - -icebird 0.8.21 (published 2026-08-13) closed items 4 and the `foldCast` -object case, and added NULL guards. It did not adopt De Morgan, and its guard -targets agreement with squirreling's *engine*, which is itself two-valued, -rather than agreement with SQL. That is a coherent goal, and it is not this -repo's: #743 deliberately diverges from the engine to be SQL-correct. +same De Morgan comments. They then drifted in *opposite* directions: + +- **icebird went ahead on folding.** hypscope's sessions surface bounds its + day windows with typed literals (`TIMESTAMP '2026-08-11T00:00:00Z'`), which + squirreling parses as a `cast` node wrapping a string literal. icebird + constant-folds that shape (`staticLiteral` / `foldCast`); the kernel's copy + required a bare `literal` operand, and because AND is all-or-nothing the + whole predicate declined, so every timestamp-bounded query scanned the + cache tier unpruned. Measured on the production central server, org + `hyperparam`, 2026-08-12: 11.4s bounded on `message_created_at` versus + 7.3s bounded on `date`, same rows, same projection. icebird also gated the + boolean-position cast unwrap that let `WHERE CAST(a = 1 AS TEXT)` push a + filter that drops rows. +- **The kernel went ahead on NULLs.** LLP 0098 lets a converted filter claim + `appliedWhere`; the engine never re-filters a claimed predicate, so a + filter that disagrees with SQL is a wrong answer, not a lost optimisation. + Issues #728 and #734 (PRs #730, #743) fixed four NULL disagreements in the + kernel's copy that icebird retained: leaking relational bounds, `$nor` as a + two-valued complement of a negated OR, an unguarded `$nin`, and NULL + literals answered with `IS NULL` semantics. + +Adopting either copy as-was meant losing the other's fixes. Measured on a +24-predicate battery over a nullable column, evaluated through hyparquet's +`matchFilter` against SQL three-valued truth: the kernel's converter was +wrong on 0, `icebird@0.8.21` on 11. ## Decision {#decision} -**The kernel keeps its converter, and ports the fold.** `staticLiteral`, -`foldCast` and `castTimestamp` come across from icebird, along with the -`TRUTHINESS_PRESERVING_CASTS` gate on the boolean-position cast unwrap. The -NULL handling stays as #730 and #743 left it. - -A folded bound is guarded exactly like a plain one: `at >= TIMESTAMP '...'` -converts to `{at: {$ne: null, $gte: }}`. Folding decides *whether* a -bound pushes down; it does not change what NULL means. This is the property -that made the fold unlandable while the guards were missing, and landable -now that they are not. - -### `coerceBigInt` stays, for now {#bigint-coercion} - -Dropping it (so integer literals stay plain numbers) would restore bloom -pruning on INT32, FLOAT and DOUBLE columns, since hyparquet's -`hashParquetValue` rejects a bigint for those. That is a real win and it is -separable from this one: it changes the filter shape for every integer -predicate in the repo. Left as follow-up rather than bundled here. +**Fix the stack bottom-up, then re-export.** Three releases, in dependency +order, and the re-export lands only after all three: + +1. **squirreling 0.15.3** made the *engine* three-valued: comparisons with a + null operand are UNKNOWN rather than false, `NOT` keeps UNKNOWN as + UNKNOWN instead of JS `!` flipping it to true, AND/OR use Kleene logic, + and `IN` treats null members and null operands as UNKNOWN non-matches. + This closed issue #734's "option 1" for real: a *declined* predicate now + falls back to an engine that answers it correctly, so declining became a + safe move rather than a differently-wrong one. +2. **icebird 0.8.22** made the *converter* target SQL truth rather than + bug-compatibility with the old engine: De Morgan instead of `$nor` (which + also restores row-group pruning under negation), negated comparisons push + their flipped operator bare, `$ne`/`$nin` carry `$ne: null` guards, a + `NOT IN` list holding NULL converts to hyparquet's never-match, NULL + members of a plain IN list are dropped to keep statistics pruning + decidable, and NULL-literal comparisons decline to the now-three-valued + engine. The same battery: 0 wrong of 26. +3. **This repo** bumps all three pins and deletes its converter for the + re-export. + +### Floor: hyparquet 1.28.2 {#hyparquet-floor} + +icebird's converter pushes bare relational bounds (`{ts: {$lte: v}}`), which +is only correct because hyparquet >= 1.28.2's `matchFilter` rejects null +cells in `$lt`/`$lte`/`$gt`/`$gte`. On 1.28.1 those coerce a null cell to 0 +and the bound leaks NULL rows - the reason the kernel's copy carried +`$ne: null` guards. The root pin moves 1.28.1 to 1.28.2 with the same exact +pin, resolving to a single deduped copy shared with icebird. + +### `coerceBigInt` is dropped, not ported {#no-bigint-coercion} + +Integer literals stay plain numbers. `filterStrict: false` (what +`parquet-source.js` and icebird both pass) compares through `equals()`, so +`5 == 5n` holds against bigint-decoded INT64 columns, `$in`/`$nin` route +through the same `equals()` as of 1.28.1, and hyparquet's bloom hashing +rejects a bigint for INT32/FLOAT/DOUBLE - the coercion bought nothing on +INT64 and disabled bloom pruning everywhere else. ## Consequences {#consequences} -- Timestamp-bounded predicates prune the cache tier. Worth ~4s of the ~9s - sessions-list batch-0 scan measured above; the rest of that scan is a - separate matter (there is no partition-level pruning: `sql.js` calls - `discoverPartitions` with no WHERE, so every raw query still opens all 752 - cache files before any filter runs). -- The truthiness-cast pushdown bug is gone. -- The two tiers still disagree on NULLs, because the archive tier runs - icebird's converter. That gap is filed as **#744** and is not closed here. - Closing it means porting the four items above upstream, at which point the - kernel's copy could genuinely become a re-export. -- The timestamp fixture is nullable and carries a NULL row, and the day-bound - test asserts `appliedWhere` rather than only the row set. Without that - assertion an upstream regression that stopped folding typed literals would - keep the suite green and silently give back the scan time. +- Timestamp-bounded predicates prune the cache tier; the truthiness-cast + bug is gone; bloom pruning is restored for non-INT64 numerics. +- The cache tier and archive tier answer the same predicate with the same + rows. Issue #744 (the archive tier's NULL wrongness) is closed by the + same bump that lands this. +- Issue #734 is closed outright: `NOT (col LIKE 'a%')` and every other + negation of a declined subtree is now answered correctly by the engine. +- Future converter fixes land once, in icebird, and both tiers get them. The + guardrail against silent regression is behavioral, not structural: + `test/core/parquet-source.test.js` asserts SQL row sets end to end through + real parquet scans (nullable TIMESTAMP fixture included), asserts + `appliedWhere` so a fold regression cannot silently hand the work back to + the engine, and measures bytes read so a pruning regression fails loudly. +- The unit shape assertions now document icebird's shapes: bare relational + bounds, guards only on `$ne`/`$nin`, declines for NULL-literal + comparisons. diff --git a/package.json b/package.json index f8262a51..748db671 100644 --- a/package.json +++ b/package.json @@ -69,11 +69,11 @@ "dependencies": { "@aws-sdk/client-s3": "3.1109.0", "@aws-sdk/credential-provider-ini": "3.973.13", - "hyparquet": "1.28.1", + "hyparquet": "1.28.2", "hyparquet-compressors": "1.1.1", - "icebird": "0.8.20", + "icebird": "0.8.22", "marked": "18.0.9", - "squirreling": "0.15.2" + "squirreling": "0.15.3" }, "optionalDependencies": { "hyparquet-writer": "0.16.6", diff --git a/src/core/query/parquet-pushdown.js b/src/core/query/parquet-pushdown.js index 516a3cf6..18e408af 100644 --- a/src/core/query/parquet-pushdown.js +++ b/src/core/query/parquet-pushdown.js @@ -1,417 +1,25 @@ // @ts-check -/** - * Convert a squirreling `WHERE` clause AST into a hyparquet - * `ParquetQueryFilter` (a MongoDB-style predicate) so the scan can push - * the predicate down to the parquet reader. Returns `undefined` whenever - * the expression cannot be fully and faithfully converted. The caller - * must then leave `appliedWhere` false and let the SQL engine filter the - * rows itself. Note that the engine's own filter is two-valued for NULLs, - * so declining is a correctness *fallback*, not a correctness *guarantee*: - * on a nullable column it can still return rows SQL's three-valued logic - * excludes, and it does so for every negation of an UNKNOWN subtree - * (`NOT (col LIKE 'a%')` over a nullable column, issue #734). Prefer a - * faithful filter over a decline where one exists: a predicate that is - * UNKNOWN for every row is faithfully pushable as hyparquet's never-match - * even though it looks like nothing worth pushing. - * - * Ported from the Hyperparam app (`lib/tools/parquetPushdownFilter.ts`), - * which drives the same squirreling + hyparquet stack. The node-type - * discriminants match `squirreling@0.12` (`unary`, `binary`, - * `in valuelist`, `cast`, `identifier`, `literal`). - * - * @import { BinaryNode, BinaryOp, CastType, ComparisonOp, ExprNode, InValuesNode, SqlPrimitive } from 'squirreling/src/types.js' - * @import { ParquetQueryFilter } from 'hyparquet' - */ - -/** - * @param {ExprNode | undefined} where - * @returns {ParquetQueryFilter | undefined} - */ -export function whereToParquetFilter(where) { - if (!where) return undefined - return convertExpr(where, false) -} - -/** - * @param {ExprNode} node - * @param {boolean} negate - * @returns {ParquetQueryFilter | undefined} - */ -function convertExpr(node, negate) { - if (node.type === 'unary' && node.op === 'NOT') { - return convertExpr(node.argument, !negate) - } - if (node.type === 'unary' && (node.op === 'IS NULL' || node.op === 'IS NOT NULL')) { - if (node.argument.type !== 'identifier') return undefined - const isNull = (node.op === 'IS NULL') !== negate - return { [node.argument.name]: { [isNull ? '$eq' : '$ne']: null } } - } - if (node.type === 'binary') { - return convertBinary(node, negate) - } - if (node.type === 'in valuelist') { - return convertInValues(node, negate) - } - if (node.type === 'cast' && TRUTHINESS_PRESERVING_CASTS.has(node.toType)) { - // A cast at boolean position (`WHERE CAST(a = 1 AS INT)`) keeps its - // operand's truthiness only for boolean and numeric targets. TEXT does - // not: the engine renders `a = 1` to the string `'false'`, which is - // truthy, so unwrapping it pushed a filter that drops rows the query - // selects. TIMESTAMP does not either, since every Date is truthy. Both - // fall through to the engine. - return convertExpr(node.expr, negate) - } - // Non-convertible node types (functions, subqueries, CASE, …) fall - // through to undefined so the engine applies the predicate itself. - return undefined -} - -/** - * The cast targets whose result is truthy exactly when their operand is, and - * so may be unwrapped at boolean position. Complement of the four types - * squirreling's `isCastType` adds: TEXT, STRING, VARCHAR and TIMESTAMP. - * - * @type {Set} - */ -const TRUTHINESS_PRESERVING_CASTS = new Set( - ['BOOLEAN', 'BOOL', 'INTEGER', 'INT', 'BIGINT', 'FLOAT', 'REAL', 'DOUBLE'] -) - -/** - * @ref LLP 0098 [constrained-by]: the engine trusts `appliedWhere` and never - * re-judges a claimed predicate, so a predicate that matches nothing is worth - * converting rather than declining: the decline is not a no-op, it is a - * handoff to a two-valued filter that answers the negation wrong. - * - * @param {BinaryNode} node - * @param {boolean} negate - * @returns {ParquetQueryFilter | undefined} - */ -function convertBinary(node, negate) { - const { op, left, right } = node - if (op === 'AND') { - const leftFilter = convertExpr(left, negate) - const rightFilter = convertExpr(right, negate) - if (!leftFilter || !rightFilter) return undefined - // De Morgan: NOT (a AND b) === (NOT a) OR (NOT b) - return negate ? { $or: [leftFilter, rightFilter] } : { $and: [leftFilter, rightFilter] } - } - if (op === 'OR') { - // De Morgan: NOT (a OR b) === (NOT a) AND (NOT b), which holds in Kleene - // three-valued logic too. The obvious `$nor` wrapper does not: hyparquet - // evaluates it as a two-valued complement, reporting "no child matched" - // as a match, so a row that is UNKNOWN for every disjunct sails past - // every leaf guard. Pushing the negation into the children instead lets - // each leaf carry its own `$ne: null`, and `$and` prunes on row-group - // statistics where `$nor` never can. - const leftFilter = convertExpr(left, negate) - const rightFilter = convertExpr(right, negate) - if (!leftFilter || !rightFilter) return undefined - return negate ? { $and: [leftFilter, rightFilter] } : { $or: [leftFilter, rightFilter] } - } - const { column, value, flipped } = extractColumnAndValue(left, right) - if (column === undefined || value === undefined) return undefined - // A comparison against a NULL literal (`col = NULL`, `col < NULL`, - // `NULL >= col`, `col LIKE NULL`) is UNKNOWN for every row under - // three-valued logic: no row is TRUE, NULL rows included, and no amount of - // negation rescues one, since `NOT UNKNOWN` is UNKNOWN. So it matches - // nothing whatever `negate` says, which is `$in: []`, the same never-match - // `convertInValues` pushes for `col NOT IN (…, NULL)` (see there for why - // hyparquet reads an empty `$in` as "no row, no row group"). - // - // Pushing beats declining even where the engine happens to agree. Its WHERE - // is two-valued (a comparison with a NULL operand is FALSE, not UNKNOWN, - // and unary `NOT` is JS `!`), so it answers `col = NULL` right by accident - // and every negation of it wrong: `NOT (col = NULL)` returned every row - // (issue #734). Pushing also prunes on row-group statistics, which the - // fallback never can. `IS NULL`, the predicate this shape gets mistaken - // for, goes through the unary path and is unaffected. - // - // Only comparisons and LIKE take this branch. Arithmetic and `||` against a - // NULL literal are never TRUE either, but they are values rather than - // predicates, and a WHERE made of one is exotic enough not to widen the - // claim for. - if (value === null) { - if (!isComparisonOp(op) && op !== 'LIKE') return undefined - return { [column]: { $in: [] } } - } - // LIKE against anything else has no parquet-filter equivalent; let the - // engine handle it. That fallback is only NULL-correct while the LIKE is - // not negated (issue #734 tracks the rest, which needs three-valued logic - // in the engine itself). - if (op === 'LIKE') return undefined - - const mongoOp = mapOperator(op, flipped, negate) - if (!mongoOp) return undefined - return guardNulls(column, mongoOp, value) -} - -/** - * Add the NULL guard a relational or inequality operator needs. - * - * hyparquet's `matchFilter` evaluates `$lt`/`$lte`/`$gt`/`$gte` with raw - * JavaScript relational operators, which coerce a NULL column value to `0`: - * `null <= 300n` and `null > -400n` are both true, so NULL rows sail past a - * bare bound (`>` and `>=` only look safe because a positive bound beats 0). - * `$ne` negates a failed equality, so NULL passes it too. SQL three-valued - * logic rejects every one of those rows. `convertInValues` applies the same - * guard to `$in`/`$nin` for the same reason. - * - * The guard is a `$ne: null` conjunct, and it rides inside the same condition - * object rather than an outer `$and` because `canSkipRowGroup` and - * `filterPageRanges` disable statistics pruning for any condition a NULL - * value could satisfy: a bare `{col: {$lte: v}}` reads as NULL-matching and - * forfeits row-group and page skipping on every chunk holding a NULL, and so - * does `{$and: [{col: {$ne: null}}, {col: {$lte: v}}]}`, whose bound is still - * bare inside its own branch. `{col: {$ne: null, $lte: v}}` prunes. - * - * `$ne` is the one operator whose guard key collides with its own, so it - * takes the `$and` form. That costs no pruning in practice: hyparquet only - * skips on `$ne` when a chunk is constant at the excluded value, and it - * already declines to skip such a chunk once the column has NULLs in it. - * - * `$eq` needs no guard: `equals(null, )` is already false, and - * `$eq: null` is exactly how the `IS NULL` path spells itself. - * - * A per-leaf guard is only sound because every negation is pushed down to a - * leaf: `convertBinary` uses De Morgan for both `AND` and `OR`, and each leaf - * absorbs the negation itself (`mapOperator` for comparisons, `convertExpr` - * for `IS NULL` / `IS NOT NULL`, `convertInValues` for `IN`). Any wrapper - * that complements a subtree wholesale, such as `$nor`, evaluates two-valued - * and hands back the rows its children left UNKNOWN, defeating the guards - * underneath it. Keep negation at the leaves. - * - * @ref LLP 0098 [constrained-by]: pushdown may only claim - * `appliedWhere` for a filter that is faithful to SQL semantics; the engine - * never re-filters a claimed predicate, so a leak here is a wrong answer. - * - * @param {string} column - * @param {'$lt' | '$lte' | '$gt' | '$gte' | '$eq' | '$ne'} mongoOp - * @param {SqlPrimitive} value - * @returns {ParquetQueryFilter} - */ -function guardNulls(column, mongoOp, value) { - if (mongoOp === '$eq') return { [column]: { $eq: value } } - if (mongoOp === '$ne') return { $and: [{ [column]: { $ne: null } }, { [column]: { $ne: value } }] } - return { [column]: { $ne: null, [mongoOp]: value } } -} - -/** - * Pull a `column op literal` (or `literal op column`) shape out of a - * binary node's operands. Returns `flipped: true` when the literal was - * on the left so the caller can mirror the comparison operator. - * - * @param {ExprNode} left - * @param {ExprNode} right - * @returns {{ column: string | undefined, value: SqlPrimitive | undefined, flipped: boolean }} - */ -function extractColumnAndValue(left, right) { - if (left.type === 'identifier') { - const lit = staticLiteral(right) - if (lit) return { column: left.name, value: coerceBigInt(lit.value), flipped: false } - } else if (right.type === 'identifier') { - const lit = staticLiteral(left) - if (lit) return { column: right.name, value: coerceBigInt(lit.value), flipped: true } - } - return { column: undefined, value: undefined, flipped: false } -} - -/** - * Statically evaluate an operand to a constant. Handles plain literals and - * casts wrapping them, which is the shape squirreling parses a typed literal - * into: `TIMESTAMP '2026-08-11T00:00:00Z'` is a `cast` node over a string - * literal, not a literal. Folding it is what lets a day-bounded query push - * its bound down at all; without it the bound declined, and because `AND` is - * all-or-nothing the whole predicate declined with it, so every - * timestamp-bounded query scanned unfiltered. - * - * The result is wrapped in `{value}` so a literal NULL (which the caller - * turns into a never-match) stays distinguishable from "not a constant". - * - * @ref LLP 0219#decision [implements]: the fold comes from icebird, the NULL - * semantics do not; a folded bound is guarded exactly like a plain one - * - * @param {ExprNode} node - * @returns {{ value: SqlPrimitive } | undefined} - */ -function staticLiteral(node) { - if (node.type === 'literal') return { value: node.value } - if (node.type === 'cast') { - const inner = staticLiteral(node.expr) - if (!inner) return undefined - return foldCast(node.toType, inner.value) - } - return undefined -} - -/** - * Mirror of squirreling's CAST evaluation over primitive literals. This must - * stay in lockstep with the engine: a converted filter replaces engine-side - * WHERE rather than pre-filtering for it, so a value folded differently from - * the way the engine folds it is a wrong answer, not a lost optimisation. - * Anything the engine would evaluate to null (an unparseable date, NaN) - * returns undefined so the predicate declines instead. - * - * @param {CastType} toType - * @param {SqlPrimitive} val - * @returns {{ value: SqlPrimitive } | undefined} - */ -function foldCast(toType, val) { - if (val === null || val === undefined) return undefined - if (toType === 'TEXT' || toType === 'STRING' || toType === 'VARCHAR') { - // The engine JSON-stringifies an object operand rather than calling - // String() on it, so `CAST(TIMESTAMP '...' AS TEXT)` would fold to a - // different string here than the engine produces. Its stringify helper is - // not exported, so decline rather than reimplement it. - if (typeof val === 'object') return undefined - return { value: String(val) } - } - if (toType === 'INTEGER' || toType === 'INT') { - const num = Number(val) - return isNaN(num) ? undefined : { value: Math.trunc(num) } - } - if (toType === 'BIGINT') { - if (typeof val === 'bigint') return { value: val } - const num = Number(val) - return isNaN(num) ? undefined : { value: BigInt(Math.trunc(num)) } - } - if (toType === 'FLOAT' || toType === 'REAL' || toType === 'DOUBLE') { - const num = Number(val) - return isNaN(num) ? undefined : { value: num } - } - if (toType === 'BOOLEAN' || toType === 'BOOL') { - return { value: Boolean(val) } - } - if (toType === 'TIMESTAMP') { - const date = castTimestamp(val) - return date ? { value: date } : undefined - } - return undefined -} - -/** - * Mirror of squirreling's TIMESTAMP cast: numbers are epoch millis, strings go - * through its `toDate`, which insists on a `YYYY-MM-DD` prefix. `toDate` is - * not in squirreling's public exports, hence the copy. - * - * @param {SqlPrimitive} val - * @returns {Date | undefined} - */ -function castTimestamp(val) { - if (val instanceof Date) return val - if (typeof val === 'number' || typeof val === 'bigint') { - const date = new Date(Number(val)) - return isNaN(date.getTime()) ? undefined : date - } - if (typeof val === 'string' && /^\d{4}-\d{2}-\d{2}(T\d{2}:\d{2}:\d{2})?/.test(val)) { - const date = new Date(val) - if (!isNaN(date.getTime())) return date - } - return undefined -} - -/** - * @param {BinaryOp} op - * @param {boolean} flipped - * @param {boolean} negate - * @returns {'$lt' | '$lte' | '$gt' | '$gte' | '$eq' | '$ne' | undefined} - */ -function mapOperator(op, flipped, negate) { - if (!isComparisonOp(op)) return undefined - let mapped = op - if (negate) mapped = neg(mapped) - if (flipped) mapped = flip(mapped) - if (mapped === '<') return '$lt' - if (mapped === '<=') return '$lte' - if (mapped === '>') return '$gt' - if (mapped === '>=') return '$gte' - if (mapped === '=' || mapped === '==') return '$eq' - return '$ne' -} - -/** - * @param {ComparisonOp} op - * @returns {ComparisonOp} - */ -function neg(op) { - if (op === '<') return '>=' - if (op === '<=') return '>' - if (op === '>') return '<=' - if (op === '>=') return '<' - if (op === '=' || op === '==') return '!=' - // negation of `!=` / `<>` is equality - return '=' -} - -/** - * @param {ComparisonOp} op - * @returns {ComparisonOp} - */ -function flip(op) { - if (op === '<') return '>' - if (op === '<=') return '>=' - if (op === '>') return '<' - if (op === '>=') return '<=' - return op -} - -/** - * @param {string} op - * @returns {op is ComparisonOp} - */ -function isComparisonOp(op) { - return op === '=' || op === '==' || op === '!=' || op === '<>' || op === '<' || op === '>' || op === '<=' || op === '>=' -} - -/** - * Coerce integer literals to `bigint` so they compare equal to parquet - * INT64 columns, which hyparquet decodes as `bigint`. Non-integer and - * non-number values pass through unchanged. - * - * @param {SqlPrimitive} value - * @returns {SqlPrimitive} - */ -function coerceBigInt(value) { - if (typeof value === 'number' && Number.isInteger(value)) return BigInt(value) - return value -} - -/** - * @param {InValuesNode} node - * @param {boolean} negate - * @returns {ParquetQueryFilter | undefined} - */ -function convertInValues(node, negate) { - if (node.expr.type !== 'identifier') return undefined - /** @type {SqlPrimitive[]} */ - const values = [] - for (const val of node.values) { - const lit = staticLiteral(val) - if (!lit) return undefined - values.push(coerceBigInt(lit.value)) - } - // `col NOT IN (…, NULL)` matches no row: it is FALSE for a row equal to one - // of the listed values and UNKNOWN for every other row (no value can be - // proven distinct from NULL), so no row is TRUE. `$in: []` is hyparquet's - // never-match: `matchesIn` folds an empty target list to `[].some(…)`, which - // is false, and `canSkipStats`'s `$in` branch folds it to `[].every(…)`, - // which is true, so every row group is skipped on statistics alone. Pushing - // it is also what keeps the answer right: squirreling's own `WHERE` is - // two-valued for NULLs, so handing the predicate back returns the very rows - // it excludes. - if (negate && values.some((value) => value === null)) return { [node.expr.name]: { $in: [] } } - // A NULL member of a list that is NOT negated is dropped instead. It can - // never make the disjunction TRUE (`col = NULL` is UNKNOWN for every row) - // and the guard below already excludes NULL rows, so the row set is - // identical either way, but carrying it costs pruning: `canSkipStats` orders - // every `$in` member against the chunk bounds through `compareParquetValues`, - // which returns `undefined` for a non-string against a BYTE_ARRAY bound, and - // one undecidable member makes the whole `every` fail. A string column - // filtered on `IN ('zz', NULL)` therefore reads every row group it could - // have skipped. When every member is NULL the list drops to `$in: []`, which - // is exactly what `col IN (NULL)` means. (Nothing is dropped on the negated - // path: a negated list holding a NULL returned above.) - const pushed = values.filter((value) => value !== null) - return { [node.expr.name]: { $ne: null, [negate ? '$nin' : '$in']: pushed } } -} +// icebird's converter, re-exported rather than reimplemented. This module and +// `icebird/src/sql/whereFilter.js` both began as ports of the Hyperparam app's +// `lib/tools/parquetPushdownFilter.ts` and drifted in opposite directions: +// icebird gained constant-folding of typed literals (`TIMESTAMP '...'` bounds, +// which squirreling parses as a cast over a string literal) while this copy +// declined them and pushed nothing down; this copy gained SQL three-valued +// NULL semantics (#728, #730, #734, #743) while icebird's stayed wrong on +// nullable columns. Neither copy was adoptable by the other until the NULL +// work converged: squirreling >= 0.15.3 evaluates WHERE with Kleene +// three-valued logic, and icebird >= 0.8.22 pushes filters that agree with it +// (De Morgan instead of `$nor`, `$ne`/`$nin` null guards, never-match for a +// NOT IN list holding NULL, declines answered by the now-three-valued engine). +// +// Floor: hyparquet >= 1.28.2, whose `matchFilter` rejects null cells in the +// bare relational operators icebird emits ($lt/$lte/$gt/$gte). On 1.28.1 +// those coerce a null cell to 0 and a bare bound leaks NULL rows, which is +// why the kernel's copy carried its own `$ne: null` guards. The floor also +// covers 1.28.1's `$in`/`$nin` matching through `equals()` rather than +// `Array.prototype.includes`, so plain-number literals match bigint-decoded +// INT64 columns and the old `coerceBigInt` shim is unnecessary. +// +// @ref LLP 0219 [implements]: one pushdown converter for the whole stack, owned by icebird +export { whereToParquetFilter } from 'icebird/src/sql/whereFilter.js' diff --git a/test/core/parquet-source.test.js b/test/core/parquet-source.test.js index e57fcf3d..dbc82775 100644 --- a/test/core/parquet-source.test.js +++ b/test/core/parquet-source.test.js @@ -148,30 +148,37 @@ async function run(source, query) { // --- pushdown conversion ----------------------------------------------------- -test('whereToParquetFilter converts simple comparisons (integers coerced to bigint)', () => { - assert.deepEqual(whereToParquetFilter(whereOf('SELECT * FROM t WHERE id = 3')), { id: { $eq: 3n } }) - assert.deepEqual(whereToParquetFilter(whereOf('SELECT * FROM t WHERE id > 3')), { id: { $ne: null, $gt: 3n } }) - assert.deepEqual(whereToParquetFilter(whereOf('SELECT * FROM t WHERE id <= 3')), { id: { $ne: null, $lte: 3n } }) +// Integer literals stay plain numbers: hyparquet >= 1.28.2 compares them to +// bigint-decoded INT64 columns through `equals()`, and its bloom hashing +// rejects a bigint for INT32/FLOAT/DOUBLE, so coercing would cost pruning. +// Relational bounds push bare: 1.28.2's matchFilter rejects null cells in +// $lt/$lte/$gt/$gte, so no guard is needed (LLP 0219). +test('whereToParquetFilter converts simple comparisons', () => { + assert.deepEqual(whereToParquetFilter(whereOf('SELECT * FROM t WHERE id = 3')), { id: { $eq: 3 } }) + assert.deepEqual(whereToParquetFilter(whereOf('SELECT * FROM t WHERE id > 3')), { id: { $gt: 3 } }) + assert.deepEqual(whereToParquetFilter(whereOf('SELECT * FROM t WHERE id <= 3')), { id: { $lte: 3 } }) assert.deepEqual(whereToParquetFilter(whereOf("SELECT * FROM t WHERE name = 'bob'")), { name: { $eq: 'bob' } }) }) test('whereToParquetFilter mirrors flipped operands (literal on the left)', () => { - assert.deepEqual(whereToParquetFilter(whereOf('SELECT * FROM t WHERE 3 < id')), { id: { $ne: null, $gt: 3n } }) - assert.deepEqual(whereToParquetFilter(whereOf('SELECT * FROM t WHERE 3 >= id')), { id: { $ne: null, $lte: 3n } }) + assert.deepEqual(whereToParquetFilter(whereOf('SELECT * FROM t WHERE 3 < id')), { id: { $gt: 3 } }) + assert.deepEqual(whereToParquetFilter(whereOf('SELECT * FROM t WHERE 3 >= id')), { id: { $lte: 3 } }) }) test('whereToParquetFilter handles AND / OR / NOT', () => { assert.deepEqual( whereToParquetFilter(whereOf('SELECT * FROM t WHERE id >= 2 AND id <= 4')), - { $and: [{ id: { $ne: null, $gte: 2n } }, { id: { $ne: null, $lte: 4n } }] } + { $and: [{ id: { $gte: 2 } }, { id: { $lte: 4 } }] } ) assert.deepEqual( whereToParquetFilter(whereOf('SELECT * FROM t WHERE id = 1 OR id = 2')), - { $or: [{ id: { $eq: 1n } }, { id: { $eq: 2n } }] } + { $or: [{ id: { $eq: 1 } }, { id: { $eq: 2 } }] } ) + // $ne is true on a null cell in hyparquet (MongoDB semantics), so it is the + // one comparison that carries a null guard assert.deepEqual( whereToParquetFilter(whereOf('SELECT * FROM t WHERE NOT (id = 1)')), - { $and: [{ id: { $ne: null } }, { id: { $ne: 1n } }] } + { $and: [{ id: { $ne: null } }, { id: { $ne: 1 } }] } ) // De Morgan: NOT (a OR b) -> $and of the negated children, never `$nor`, // whose two-valued complement matches the rows its children left UNKNOWN @@ -179,21 +186,23 @@ test('whereToParquetFilter handles AND / OR / NOT', () => { whereToParquetFilter(whereOf('SELECT * FROM t WHERE NOT (id = 1 OR id = 2)')), { $and: [ - { $and: [{ id: { $ne: null } }, { id: { $ne: 1n } }] }, - { $and: [{ id: { $ne: null } }, { id: { $ne: 2n } }] }, + { $and: [{ id: { $ne: null } }, { id: { $ne: 1 } }] }, + { $and: [{ id: { $ne: null } }, { id: { $ne: 2 } }] }, ], } ) }) test('whereToParquetFilter handles IN / NOT IN / IS NULL', () => { + // $in never matches a null cell, so it pushes bare; $nin, like $ne, is + // true on one, so it carries the guard assert.deepEqual( whereToParquetFilter(whereOf('SELECT * FROM t WHERE id IN (1, 2)')), - { id: { $ne: null, $in: [1n, 2n] } } + { id: { $in: [1, 2] } } ) assert.deepEqual( whereToParquetFilter(whereOf('SELECT * FROM t WHERE id NOT IN (1, 2)')), - { id: { $ne: null, $nin: [1n, 2n] } } + { $and: [{ id: { $ne: null } }, { id: { $nin: [1, 2] } }] } ) assert.deepEqual(whereToParquetFilter(whereOf('SELECT * FROM t WHERE name IS NULL')), { name: { $eq: null } }) assert.deepEqual(whereToParquetFilter(whereOf('SELECT * FROM t WHERE name IS NOT NULL')), { name: { $ne: null } }) @@ -201,13 +210,11 @@ test('whereToParquetFilter handles IN / NOT IN / IS NULL', () => { // The regression that motivated LLP 0219: squirreling parses a typed literal // as a cast over a string, and requiring a bare literal operand made every -// timestamp-bounded predicate convert to undefined and prune nothing. The -// folded bound still carries the same `$ne: null` guard a plain bound does: -// folding decides *whether* the bound pushes down, not what NULL means. +// timestamp-bounded predicate convert to undefined and prune nothing. test('whereToParquetFilter folds typed literals (TIMESTAMP casts)', () => { assert.deepEqual( whereToParquetFilter(whereOf("SELECT * FROM t WHERE at >= TIMESTAMP '2026-08-11T00:00:00Z'")), - { at: { $ne: null, $gte: new Date('2026-08-11T00:00:00Z') } } + { at: { $gte: new Date('2026-08-11T00:00:00Z') } } ) // AND is all-or-nothing, so a day window only converts if both sides do assert.deepEqual( @@ -216,8 +223,8 @@ test('whereToParquetFilter folds typed literals (TIMESTAMP casts)', () => { )), { $and: [ - { at: { $ne: null, $gte: new Date('2026-08-11T00:00:00Z') } }, - { at: { $ne: null, $lt: new Date('2026-08-12T00:00:00Z') } }, + { at: { $gte: new Date('2026-08-11T00:00:00Z') } }, + { at: { $lt: new Date('2026-08-12T00:00:00Z') } }, ], } ) @@ -229,7 +236,7 @@ test('whereToParquetFilter folds typed literals (TIMESTAMP casts)', () => { // truthiness. CAST( AS TEXT) yields 'false', which is truthy, so pushing // the bare comparison down would drop rows the query selects. test('whereToParquetFilter only unwraps truthiness-preserving casts', () => { - assert.deepEqual(whereToParquetFilter(whereOf('SELECT * FROM t WHERE CAST(id = 1 AS INT)')), { id: { $eq: 1n } }) + assert.deepEqual(whereToParquetFilter(whereOf('SELECT * FROM t WHERE CAST(id = 1 AS INT)')), { id: { $eq: 1 } }) assert.equal(whereToParquetFilter(whereOf('SELECT * FROM t WHERE CAST(id = 1 AS TEXT)')), undefined) }) @@ -240,23 +247,28 @@ test('whereToParquetFilter returns undefined for non-convertible predicates', () assert.equal(whereToParquetFilter(undefined), undefined) }) -test('whereToParquetFilter handles predicates whose SQL result is always UNKNOWN', () => { - // Comparison against a NULL literal never matches a row, not even a NULL - // one, and `NOT UNKNOWN` is still UNKNOWN, so the whole family pushes - // `$in: []`, hyparquet's never-match. Declining instead would hand the - // predicate to squirreling's two-valued WHERE, which returns every row for - // the negated shapes (issue #734). - assert.deepEqual(whereToParquetFilter(whereOf('SELECT * FROM t WHERE id = NULL')), { id: { $in: [] } }) - assert.deepEqual(whereToParquetFilter(whereOf('SELECT * FROM t WHERE id != NULL')), { id: { $in: [] } }) - assert.deepEqual(whereToParquetFilter(whereOf('SELECT * FROM t WHERE id < NULL')), { id: { $in: [] } }) - assert.deepEqual(whereToParquetFilter(whereOf('SELECT * FROM t WHERE NULL >= id')), { id: { $in: [] } }) - assert.deepEqual(whereToParquetFilter(whereOf('SELECT * FROM t WHERE NOT (id = NULL)')), { id: { $in: [] } }) - assert.deepEqual(whereToParquetFilter(whereOf('SELECT * FROM t WHERE NOT NOT (id = NULL)')), { id: { $in: [] } }) - assert.deepEqual(whereToParquetFilter(whereOf('SELECT * FROM t WHERE name LIKE NULL')), { name: { $in: [] } }) - assert.deepEqual(whereToParquetFilter(whereOf('SELECT * FROM t WHERE NOT (name LIKE NULL)')), { name: { $in: [] } }) - // NOT IN over a list containing NULL matches no row either: `$in: []` is - // hyparquet's never-match, and pushing it beats declining, whose fallback - // (squirreling's two-valued WHERE) returns rows. +test('whereToParquetFilter declines NULL-literal comparisons to the engine', () => { + // A comparison against a NULL literal is UNKNOWN for every row. icebird + // declines it rather than pushing a filter ({$eq: null} would mean IS NULL + // to hyparquet), and squirreling >= 0.15.3 answers the fallback with + // three-valued logic, so the negated shapes that issue #734 caught + // returning every row now correctly return none (asserted end to end + // below). + assert.equal(whereToParquetFilter(whereOf('SELECT * FROM t WHERE id = NULL')), undefined) + assert.equal(whereToParquetFilter(whereOf('SELECT * FROM t WHERE id != NULL')), undefined) + assert.equal(whereToParquetFilter(whereOf('SELECT * FROM t WHERE id < NULL')), undefined) + assert.equal(whereToParquetFilter(whereOf('SELECT * FROM t WHERE NULL >= id')), undefined) + assert.equal(whereToParquetFilter(whereOf('SELECT * FROM t WHERE NOT (id = NULL)')), undefined) + assert.equal(whereToParquetFilter(whereOf('SELECT * FROM t WHERE NOT (id + 1 = NULL)')), undefined) + assert.equal(whereToParquetFilter(whereOf('SELECT * FROM t WHERE id + NULL')), undefined) + // A declined conjunct collapses the surrounding tree to the engine too + assert.equal(whereToParquetFilter(whereOf('SELECT * FROM t WHERE id = NULL OR id = 3')), undefined) +}) + +test('whereToParquetFilter handles NULL members of an IN list', () => { + // NOT IN over a list containing NULL matches no row: FALSE on a listed + // value, UNKNOWN everywhere else, and no negation rescues an UNKNOWN. + // `$in: []` is hyparquet's never-match and prunes every row group. assert.deepEqual( whereToParquetFilter(whereOf('SELECT * FROM t WHERE id NOT IN (1, NULL)')), { id: { $in: [] } } @@ -265,50 +277,21 @@ test('whereToParquetFilter handles predicates whose SQL result is always UNKNOWN // it is dropped: same rows, and the leaf keeps its statistics pruning. assert.deepEqual( whereToParquetFilter(whereOf('SELECT * FROM t WHERE id IN (1, NULL)')), - { id: { $ne: null, $in: [1n] } } + { id: { $in: [1] } } ) assert.deepEqual( whereToParquetFilter(whereOf('SELECT * FROM t WHERE id IN (NULL)')), - { id: { $ne: null, $in: [] } } - ) -}) - -test('whereToParquetFilter pushes never-match only where the shape proves it', () => { - // A comparison against a non-NULL literal keeps its ordinary filter: the - // never-match is for UNKNOWN-for-every-row, not for "has a NULL somewhere". - assert.deepEqual( - whereToParquetFilter(whereOf('SELECT * FROM t WHERE NOT (id = 3)')), - { $and: [{ id: { $ne: null } }, { id: { $ne: 3n } }] } - ) - // A never-match leaf composes: it zeroes an AND and leaves an OR to its - // sibling, which is what Kleene logic says (UNKNOWN AND x is never TRUE, - // UNKNOWN OR x is TRUE exactly where x is). - assert.deepEqual( - whereToParquetFilter(whereOf('SELECT * FROM t WHERE id = NULL OR id = 3')), - { $or: [{ id: { $in: [] } }, { id: { $eq: 3n } }] } + { id: { $in: [] } } ) - // The NULL literal has to sit opposite a plain column. Against an - // expression there is no leaf to name, and against another literal there is - // no column at all, so both keep declining. - assert.equal(whereToParquetFilter(whereOf('SELECT * FROM t WHERE NOT (id + 1 = NULL)')), undefined) - assert.equal(whereToParquetFilter(whereOf('SELECT * FROM t WHERE NOT (NULL = 1)')), undefined) - // The branch is for predicates, not values: an arithmetic or concat - // expression against a NULL literal still declines even though its column - // is a bare identifier. - assert.equal(whereToParquetFilter(whereOf('SELECT * FROM t WHERE id + NULL')), undefined) - assert.equal(whereToParquetFilter(whereOf('SELECT * FROM t WHERE name || NULL')), undefined) - // ...and a negated LIKE over a real pattern stays declined (and, on a - // nullable column, stays SQL-wrong until the engine speaks three-valued - // logic: issue #734, option 1). - assert.equal(whereToParquetFilter(whereOf("SELECT * FROM t WHERE NOT (name LIKE 'a%')")), undefined) }) // --- NULL rows must not leak past a pushed-down filter ------------------------ // @ref LLP 0098 [tests]: the scan claims `appliedWhere` for -// every convertible predicate, so the engine never re-filters. hyparquet -// evaluates a bare bound with raw JS comparison, where `null <= 300n` is true, -// so an unguarded filter is a silent wrong answer rather than an error. +// every convertible predicate, so the engine never re-filters, and a filter +// that disagrees with SQL on null cells is a silent wrong answer rather than +// an error. hyparquet >= 1.28.2 rejects null cells in bare relational bounds; +// $ne and $nin need the converter's explicit guard. test('pushed-down comparisons do not leak NULL rows (issue #728)', async () => { /** @type {[string, number[]][]} */ const cases = [ @@ -369,12 +352,11 @@ test('comparison against a NULL literal matches no rows (issue #728)', async () assert.deepEqual(await mismatches(cases), []) }) -// A predicate the converter declines falls back to squirreling's WHERE, which -// is two-valued: a comparison with a NULL operand is FALSE rather than -// UNKNOWN, and unary NOT is JS `!`, so `NOT ` comes back TRUE. Every -// negation of a NULL-literal comparison therefore returned rows SQL excludes. -// These shapes are UNKNOWN for every row whatever the negation depth, so the -// converter pushes hyparquet's never-match instead of declining. +// A NULL-literal comparison is UNKNOWN for every row whatever the negation +// depth. The converter declines these shapes, and the decline is only safe +// because squirreling >= 0.15.3 evaluates WHERE with three-valued logic: +// its old two-valued NOT flipped UNKNOWN to TRUE and returned every row for +// exactly these predicates (issue #734). test('negated comparisons against a NULL literal match no rows (issue #734)', async () => { /** @type {[string, number[]][]} */ const cases = [ From 0c6af6dc9db8962ccecf344da84a25b5df272ca4 Mon Sep 17 00:00:00 2001 From: Kenny Daniel Date: Thu, 13 Aug 2026 14:11:30 -0700 Subject: [PATCH 3/3] Renumber LLP 0219 to 0222; 0219 is taken on master --- ...er.decision.md => 0222-one-pushdown-converter.decision.md} | 2 +- src/core/query/parquet-pushdown.js | 2 +- test/core/parquet-source.test.js | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) rename llp/{0219-one-pushdown-converter.decision.md => 0222-one-pushdown-converter.decision.md} (98%) diff --git a/llp/0219-one-pushdown-converter.decision.md b/llp/0222-one-pushdown-converter.decision.md similarity index 98% rename from llp/0219-one-pushdown-converter.decision.md rename to llp/0222-one-pushdown-converter.decision.md index d4529668..5660c146 100644 --- a/llp/0219-one-pushdown-converter.decision.md +++ b/llp/0222-one-pushdown-converter.decision.md @@ -1,4 +1,4 @@ -# LLP 0219: One WHERE-to-parquet-filter converter, owned by icebird +# LLP 0222: One WHERE-to-parquet-filter converter, owned by icebird **Type:** Decision **Status:** Accepted diff --git a/src/core/query/parquet-pushdown.js b/src/core/query/parquet-pushdown.js index 18e408af..c9a85e8f 100644 --- a/src/core/query/parquet-pushdown.js +++ b/src/core/query/parquet-pushdown.js @@ -21,5 +21,5 @@ // `Array.prototype.includes`, so plain-number literals match bigint-decoded // INT64 columns and the old `coerceBigInt` shim is unnecessary. // -// @ref LLP 0219 [implements]: one pushdown converter for the whole stack, owned by icebird +// @ref LLP 0222 [implements]: one pushdown converter for the whole stack, owned by icebird export { whereToParquetFilter } from 'icebird/src/sql/whereFilter.js' diff --git a/test/core/parquet-source.test.js b/test/core/parquet-source.test.js index dbc82775..48f81e58 100644 --- a/test/core/parquet-source.test.js +++ b/test/core/parquet-source.test.js @@ -152,7 +152,7 @@ async function run(source, query) { // bigint-decoded INT64 columns through `equals()`, and its bloom hashing // rejects a bigint for INT32/FLOAT/DOUBLE, so coercing would cost pruning. // Relational bounds push bare: 1.28.2's matchFilter rejects null cells in -// $lt/$lte/$gt/$gte, so no guard is needed (LLP 0219). +// $lt/$lte/$gt/$gte, so no guard is needed (LLP 0222). test('whereToParquetFilter converts simple comparisons', () => { assert.deepEqual(whereToParquetFilter(whereOf('SELECT * FROM t WHERE id = 3')), { id: { $eq: 3 } }) assert.deepEqual(whereToParquetFilter(whereOf('SELECT * FROM t WHERE id > 3')), { id: { $gt: 3 } }) @@ -208,7 +208,7 @@ test('whereToParquetFilter handles IN / NOT IN / IS NULL', () => { assert.deepEqual(whereToParquetFilter(whereOf('SELECT * FROM t WHERE name IS NOT NULL')), { name: { $ne: null } }) }) -// The regression that motivated LLP 0219: squirreling parses a typed literal +// The regression that motivated LLP 0222: squirreling parses a typed literal // as a cast over a string, and requiring a bare literal operand made every // timestamp-bounded predicate convert to undefined and prune nothing. test('whereToParquetFilter folds typed literals (TIMESTAMP casts)', () => {