From 84f115a75aba69b81de2764d50f5eb1ef1f7b246 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 02:52:54 +0000 Subject: [PATCH] fix(driver-memory): a no-value row is not inside `$gt`/`$gte`/`$lt`/`$lte` (#13553) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The reference matcher compared a stored `null` instead of deciding first whether the comparison was meaningful. On a NUMERIC column `null` coerces to `0`, so `null >= -1` is a true comparison between two numbers and the no-value row landed inside the bound — while this package's live mingo path excluded it. The four ordering operators now exclude a no-value row on BOTH readings of "no value" (a stored `null` and an absent key), decided in the pre-switch guard beside the reading that was already handled there. `$between` is deliberately not a member of the guarded set: it decides the case itself in `valueWithinRange`, and its answer for the degenerate all-absent range is the opposite one. A no-value COMPARAND in an ordering position is excluded from the guard too, so those cells keep today's answer rather than being decided here. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01F3jdziLbAPGeceVNmSox5L --- .changeset/ordering-arms-no-value-row.md | 37 ++++++ ...y-matcher-null-value-and-comparand.test.ts | 124 ++++++++++++++++-- .../driver-memory/src/memory-matcher.ts | 59 +++++++++ 3 files changed, 211 insertions(+), 9 deletions(-) create mode 100644 .changeset/ordering-arms-no-value-row.md diff --git a/.changeset/ordering-arms-no-value-row.md b/.changeset/ordering-arms-no-value-row.md new file mode 100644 index 0000000000..735c13694d --- /dev/null +++ b/.changeset/ordering-arms-no-value-row.md @@ -0,0 +1,37 @@ +--- +"@objectstack/driver-memory": patch +--- + +fix(driver-memory): a row with no value is not inside `$gt` / `$gte` / `$lt` / `$lte` (#13553) + +The reference matcher (`memory-matcher.ts`) compared a stored `null` instead of first +deciding whether the comparison meant anything, so on a **numeric** column a no-value row +landed inside a bound: JS coerces `null` to `0`, which makes `null >= -1` a true comparison +between two numbers. Measured on `{id:'1',n:5} {id:'2',n:0} {id:'3',n:null} {id:'4'}`, the +matcher answered `['1','2','3']` for `{n: {$gte: -1}}` and `['2','3']` for `{n: {$lte: 1}}` +where this package's live mingo path answered `['1','2']` and `['2']` — so row 3 was both +greater than `-1` and less than `1` at the same time. + +The four arms now answer EXCLUDE for a no-value row, which is what the live path already +answered and what the platform's settled reading gives: only the negation-carrying operators +(`$ne` / `$nin` / `$notContains`) admit a no-value row (#5298 option A, re-affirmed +2026-08-10). Both readings of "no value" reach that one answer — a stored `null` and an +absent key — where before only the absent key was excluded, by a guard that never saw the +other reading. + +A **string** column is unaffected and was the reason three earlier cards passed over this: +`null >= '2026-07-01'` compares `0` against `NaN` and is false, so the same four arms looked +correct on every ISO-date fixture #13494, #13495 and #13549 used. + +Two things deliberately do NOT move. `$between` keeps deciding the no-value case in +`valueWithinRange`, whose answer is the opposite one (a range with both ends absent selects +the no-value rows, #13495). And a no-value **comparand** in an ordering position +(`{$gte: null}`) keeps today's answer exactly: it is the one null-comparand position the +filter contract still accepts — the 2026-08-31 ruling refused the three siblings +(`$in` / `$nin` null members, `$between` null endpoints, #13357) and #5332's landing had +already recorded this one in writing as a position no ruling covers. + +⭐ What this closes beyond the four cells: this package's live path compiles `$between` INTO +`$gte` + `$lte`, so `{n: {$between: [-1,1]}}` and `{n: {$gte: -1, $lte: 1}}` are one predicate +to it. After #13549 landed, this face answered them differently on the same row. It no longer +does. diff --git a/packages/drivers/driver-memory/src/memory-matcher-null-value-and-comparand.test.ts b/packages/drivers/driver-memory/src/memory-matcher-null-value-and-comparand.test.ts index 98e417c0fa..c55b71bfbb 100644 --- a/packages/drivers/driver-memory/src/memory-matcher-null-value-and-comparand.test.ts +++ b/packages/drivers/driver-memory/src/memory-matcher-null-value-and-comparand.test.ts @@ -1,16 +1,18 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. /** - * [#13494/#13495/#13549] The reference matcher against a null COMPARAND and a - * null VALUE — held to the live mingo path of its own package. + * [#13494/#13495/#13549/#13553] The reference matcher against a null COMPARAND + * and a null VALUE — held to the live mingo path of its own package. * - * Three cards, two roots, one file. Measured on `df18120502` before the repair: + * Four cards, three roots, one file. Measured on `df18120502` before the first + * repair, and on `8b04c75d7d` for #13553: * * | filter | reading | matcher | live path | | * |---|---|---|---|---| * | `{$eq: null}` | key ABSENT | `[]` | `['3']` | #13494 | * | `{$between: [null, null]}` | value `null` | `['1','3']` | `['3']` | #13495 | * | `{$between: ['2026-07-01','2026-07-15']}` | value `null` | `['1','2','4']` | `['1','2']` | #13549 | + * | `{$gte: -1}` on a NUMERIC column | value `null` | `['1','2','3']` | `['1','2']` | #13553 | * * ## The two roots — measured, not assumed * @@ -25,6 +27,22 @@ * loose `!=` had the right answer for both readings all along, and the proof * is `$ne`, which was already on the guard's allowlist and answered both * readings correctly throughout. + + * #13553 is the THIRD root, and it is the same DEFECT as #13549 in four arms + * the `$between` repair did not reach: `$gt` / `$gte` / `$lt` / `$lte` compared + * a stored `null` instead of deciding first whether the comparison meant + * anything. It went unseen for the length of three cards because every fixture + * they used stored ISO date STRINGS, where `null >= '2026-07-01'` compares `0` + * against `NaN` and is false — the arms look correct. On a NUMERIC column + * `null` coerces to `0` and the no-value row sits inside the bound, so the + * matcher answered that one row is both greater than `-1` and less than `1`. + * + * ⭐ The reason that mattered beyond the four cells: this package's LIVE path + * compiles `$between` INTO `$gte` + `$lte` (`memory-driver.ts`). Once #13549 + * landed, this face answered `{n: {$between: [-1, 1]}}` and + * `{n: {$gte: -1, $lte: 1}}` DIFFERENTLY on the same row, though they are one + * predicate to the live path. The `$between`-equals-its-two-bounds case below + * is what holds that closed. * * ## Why every expectation here is stated on BOTH faces * @@ -35,12 +53,21 @@ * that moved only one face would pass a one-face suite and re-open the class. * * ⚠️ #13357's `$in: [null]` / `$nin: [null]` arms are deliberately ABSENT from - * this file. They are `needs-user-decision` and sitting with the maintainer, - * and pinning their current answers here — in either direction — would - * prejudge that ruling. They were measured byte-identical across this repair - * (the guard exemption is written over the OPERATOR `$eq`, never over "the - * comparand is null", which is what keeps them out of its blast radius); the - * proof is in the PR, not in an assertion here. + * this file. They were `needs-user-decision` when #13494/#13495/#13549 landed; + * the maintainer ruled them on 2026-08-31 (option C) and the shapes are now + * REFUSED at the contract's validation entrance, with the negative pin in + * `memory-null-list-member-unreachable.test.ts`. Their cells are still not + * asserted here, now for the ruling's own reason — ⛔「不单独修一个到不了的 + * 路径」 — and they were measured byte-identical across this repair too. + * + * ⚠️ A null COMPARAND in an ORDERING position (`{$gte: null}`) is absent for + * the ORIGINAL reason, and it is the one such position the contract still + * ACCEPTS: the 2026-08-31 ruling refused the three siblings and #5332's + * landing had already recorded this one in writing as a position "no ruling + * covers". #13553's guard is scoped to leave those cells exactly where it + * found them, so pinning them here — in either direction — would prejudge a + * ruling nobody has made. The invariance is proven in the PR, not asserted + * here. */ import { describe, it, expect, beforeAll, afterAll } from 'vitest'; @@ -256,3 +283,82 @@ describe('[#13494/#13495/#13549] the ordinary vocabulary is untouched', () => { await bothFaces(nulled, NULLED_ROWS, { name: { $exists: true } }, ['1']); }); }); + +describe('[#13553] a no-value row is not inside `$gt` / `$gte` / `$lt` / `$lte`', () => { + it("the card's table, cell for cell, on the NUMERIC fixture", async () => { + // Was `['1','2','3']` / `['2','3']` on the matcher against `['1','2']` / + // `['2']` live. Row 3 (`n: null`) was answered greater than -1 AND less + // than 1 at the same time, because `null` coerces to `0`. + await bothFaces(numeric, NUMERIC_ROWS, { n: { $gte: -1 } }, ['1', '2']); + await bothFaces(numeric, NUMERIC_ROWS, { n: { $gt: -1 } }, ['1', '2']); + await bothFaces(numeric, NUMERIC_ROWS, { n: { $lte: 1 } }, ['2']); + await bothFaces(numeric, NUMERIC_ROWS, { n: { $lt: 1 } }, ['2']); + }); + + it('THE DISCRIMINATOR — the row storing `0` stays IN, on every arm', async () => { + // `{id: '2', n: 0}` is what stops this pin going vacuous. A "repair" that + // excluded everything FALSY — the shape a reader reaches for once told + // that `null` coerces to `0` — would drop this row too and still turn the + // four cells above green, because they happen not to distinguish them. + // Here they do: the no-value row leaves and the zero row stays. + await bothFaces(numeric, NUMERIC_ROWS, { n: { $gte: 0 } }, ['1', '2']); + await bothFaces(numeric, NUMERIC_ROWS, { n: { $lte: 0 } }, ['2']); + await bothFaces(numeric, NUMERIC_ROWS, { n: { $gte: -0.5 } }, ['1', '2']); + await bothFaces(numeric, NUMERIC_ROWS, { n: { $lt: 0.5 } }, ['2']); + }); + + it('the two readings of "no value" agree with EACH OTHER on all four arms', async () => { + // The matcher used to disagree with itself: a stored `null` reached the + // arm and was compared, while the same absence spelled as a MISSING key + // met the pre-switch guard and was excluded. Both readings now land on the + // one answer the ruling gives — EXCLUDE. + for (const op of ['$gt', '$gte', '$lt', '$lte'] as const) { + const bounded = { n: { [op]: 0 } }; + const withNullValue = matcherIds([{ id: 'x', n: null }], bounded); + const withMissingKey = matcherIds([{ id: 'x' }], bounded); + expect({ op, ids: withNullValue }).toEqual({ op, ids: withMissingKey }); + expect({ op, ids: withNullValue }).toEqual({ op, ids: [] }); + } + }); + + it('⭐ `$between` and its own two bounds now answer ONE row the same way', async () => { + // The reason this card exists. The live path compiles `$between` INTO + // `$gte` + `$lte`, so these two filters are one predicate to it. Between + // #13549 landing and this repair, THIS face answered them differently on + // row 3 — `$between` excluded the null-valued row while `$gte`/`$lte` + // admitted it. One face, two answers, one query. + for (const [min, max] of [[-1, 1], [0, 10], [-5, 5]] as const) { + const viaBetween = matcherIds(NUMERIC_ROWS, { n: { $between: [min, max] } }); + const viaBounds = matcherIds(NUMERIC_ROWS, { n: { $gte: min, $lte: max } }); + expect({ min, max, ids: viaBetween }).toEqual({ min, max, ids: viaBounds }); + } + await bothFaces(numeric, NUMERIC_ROWS, { n: { $gte: -1, $lte: 1 } }, ['2']); + await bothFaces(numeric, NUMERIC_ROWS, { n: { $between: [-1, 1] } }, ['2']); + }); + + it('the VALUED rows keep every answer they had — the repair moved no-value cells only', async () => { + await bothFaces(numeric, NUMERIC_ROWS, { n: { $gt: 0 } }, ['1']); + await bothFaces(numeric, NUMERIC_ROWS, { n: { $gt: 5 } }, []); + await bothFaces(numeric, NUMERIC_ROWS, { n: { $lt: 0 } }, []); + await bothFaces(numeric, NUMERIC_ROWS, { n: { $gte: 5 } }, ['1']); + }); + + it('the STRING fixture is unmoved — it agreed before, and still does', async () => { + // These four cells are why three cards passed over the defect: on strings + // the comparison against a null is false in both directions already. They + // are asserted so the repair is measured NOT to have moved them. + await bothFaces(sweep, SWEEP_ROWS, { v: { $gte: '2026-07-01' } }, ['1', '2', '3']); + await bothFaces(sweep, SWEEP_ROWS, { v: { $gt: '2026-07-01' } }, ['2', '3']); + await bothFaces(sweep, SWEEP_ROWS, { v: { $lte: '2026-07-15' } }, ['1', '2']); + await bothFaces(sweep, SWEEP_ROWS, { v: { $lt: '2026-07-15' } }, ['1']); + }); + + it('⛔ `$between` is NOT in the ordering set — its degenerate cell is unmoved', async () => { + // `$between` decides the no-value case itself, and its answer is the + // OPPOSITE one: the range whose both ends are no value selects the + // no-value rows (#13495). Adding `$between` to the guarded set would + // return false before `valueWithinRange` ran and silently move this cell. + await bothFaces(numeric, NUMERIC_ROWS, { n: { $between: [null, null] } }, ['3']); + await bothFaces(nulled, NULLED_ROWS, { name: { $between: [null, null] } }, ['3']); + }); +}); diff --git a/packages/drivers/driver-memory/src/memory-matcher.ts b/packages/drivers/driver-memory/src/memory-matcher.ts index ade9aa050d..b347fa8da1 100644 --- a/packages/drivers/driver-memory/src/memory-matcher.ts +++ b/packages/drivers/driver-memory/src/memory-matcher.ts @@ -162,6 +162,29 @@ function noValueSatisfiesNegation(op: string): boolean { return op === '$ne' || op === '$nin' || op === '$notContains'; } +/** + * [#13553] The four ORDERING operators — the ones whose arm answers a + * RELATIONAL comparison between the stored value and a comparand. + * + * A set, and consulted from the pre-switch guard, because what it selects is a + * RULING and not an implementation detail: a row whose field has NO VALUE is + * not inside `$gt` / `$gte` / `$lt` / `$lte`. Two independent authorities + * decide that and agree — this package's live mingo path excludes such a row + * (runtime truth, measured), and the platform's settled reading admits a + * no-value row only for the operators that carry a negation (`$ne` / `$nin` / + * `$notContains`, #5298 option A, re-affirmed 2026-08-10; see + * {@link noValueSatisfiesNegation}, whose membership is this set's exact + * complement on this question). + * + * ⚠️ `$between` is deliberately NOT a member, though it is a relational + * comparison too. It decides the no-value case itself, in + * {@link valueWithinRange}, and its answer is not this one: the degenerate + * range whose BOTH ends are no value SELECTS the no-value rows. Adding + * `$between` here would return `false` before that function ran and silently + * move a cell #13495 ruled. + */ +const ORDERING_OPERATORS: ReadonlySet = new Set(['$gt', '$gte', '$lt', '$lte']); + /** * [#13495/#13549] Is `value` inside the closed range `[min, max]`? * @@ -293,6 +316,42 @@ function checkCondition(value: any, condition: any): boolean { return false; } + // [#13553] The OTHER reading of "no value" — the key is present and + // holds `null` — on the four {@link ORDERING_OPERATORS}. It is decided + // here, beside the reading above, because the two readings have to land + // on ONE answer and this file's recurring defect is that they do not: + // the guard above sees only `undefined`, so a stored `null` used to + // reach the arm and be COMPARED. + // + // ⚠️ Being compared is the whole defect, and it is measured rather than + // reasoned: JS coerces `null` to `0` under a relational operator, so + // `null >= -1` is a true comparison between two NUMBERS and the + // no-value row lands inside the bound. On a STRING comparand the same + // line looks correct — `null >= '2026-07-01'` compares `0` against + // `NaN` and is false — which is why every fixture in #13494, #13495 and + // #13549 showed these four arms healthy. The repair is the one + // {@link valueWithinRange} landed for `$between`: comparability is + // decided BEFORE the comparison, never by it. + // + // Written over the OPERATOR, like the `$eq` exemption above and for the + // same reason — a rule spelled over "the value is null" alone would + // reach arms whose no-value answer is ruled elsewhere. + // + // ⛔ A no-value COMPARAND is excluded from this guard, deliberately, so + // those cells keep TODAY's answer rather than being decided here. + // `$gt: null` is the one null-comparand position the contract still + // ACCEPTS (measured at `parseFilterAST`): the 2026-08-31 ruling refused + // the three siblings — `$in` / `$nin` null members and `$between`'s + // null endpoints (#13357) — and #5332's landing had already recorded + // this position in writing as one "no ruling covers". Deciding it in an + // operator arm would pick a camp the platform declined to pick, and its + // sibling was settled by REFUSING the shape rather than by answering + // it. + if (value === null && ORDERING_OPERATORS.has(op) + && target !== null && target !== undefined) { + return false; + } + switch (op) { case '$eq': if (value != target) return false;