diff --git a/llp/0222-one-pushdown-converter.decision.md b/llp/0222-one-pushdown-converter.decision.md new file mode 100644 index 00000000..5660c146 --- /dev/null +++ b/llp/0222-one-pushdown-converter.decision.md @@ -0,0 +1,107 @@ +# LLP 0222: One WHERE-to-parquet-filter converter, owned by icebird + +**Type:** Decision +**Status:** Accepted +**Systems:** Query, Cache +**Author:** Phil / Claude +**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 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 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. 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} + +**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; 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 ba5bb5a2..c9a85e8f 100644 --- a/src/core/query/parquet-pushdown.js +++ b/src/core/query/parquet-pushdown.js @@ -1,304 +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, 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 -} - -/** - * @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' && 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)) - } - // `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 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 02353d23..48f81e58 100644 --- a/test/core/parquet-source.test.js +++ b/test/core/parquet-source.test.js @@ -84,6 +84,38 @@ async function makeSource() { return parquetDataSource(file, metadata) } +// `at` is nullable on purpose. Eleven shipped `ColumnSpec`s are nullable +// TIMESTAMP, `logs.timestamp` and `traces.startTimestamp` among them, and a +// non-null fixture cannot see whether a folded bound leaks NULL rows past a +// filter the engine will not re-check. +/** @type {ColumnSpec[]} */ +const TIMESTAMP_COLUMNS = [ + { name: 'id', type: 'INT64', nullable: false }, + { name: 'at', type: 'TIMESTAMP', nullable: true }, +] + +// 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, plus a NULL +// that SQL excludes from every bound. +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' }, + { id: 5, at: null }, +] + +/** + * @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) +} + /** * Same, over `NULLABLE_ROWS`. * @@ -116,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 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 } }) + 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 @@ -147,26 +186,60 @@ 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 } }) }) +// 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)', () => { + 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 @@ -174,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: [] } } @@ -199,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 = [ @@ -303,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 = [ @@ -539,6 +587,44 @@ 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'" + ) + // id 5 is the NULL row: UNKNOWN against both bounds, so SQL excludes it + assert.deepEqual(rows.map((r) => Number(r.id)), [2, 3]) +}) + +// The rows above come out right whether or not the bound is pushed down, since +// an unconverted predicate leaves `appliedWhere` false and the engine filters +// to the same answer. Pushing it down is the entire point of folding the typed +// literal, so assert the claim itself: without this, an upstream regression +// that stopped folding `TIMESTAMP '...'` would keep the suite green and +// silently give back the scan time. +test('a folded timestamp bound is actually pushed down, not left to the engine', async () => { + const source = await makeTimestampSource() + const scan = source.scan({ + columns: ['id'], + where: whereOf("SELECT id FROM t WHERE at >= TIMESTAMP '2026-08-11T00:00:00Z'"), + }) + assert.equal(scan.appliedWhere, true) +}) + +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, []) + // 4, not 5: the NULL row is UNKNOWN against the bound, so SQL drops it even + // though every non-null row qualifies + 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%'")