|
| 1 | +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. |
| 2 | + |
| 3 | +/** |
| 4 | + * [#6050] An `undefined` COMPARAND is refused, in every position — and `null` |
| 5 | + * keeps every one of its meanings. |
| 6 | + * |
| 7 | + * # What was measured |
| 8 | + * |
| 9 | + * On `origin/main` at `cba7454df`, one `TursoDriver` over the shared conformance |
| 10 | + * fixture (`d` valued on rows 1-2, NULL on 3-4) answered the same filter two |
| 11 | + * ways, chosen by the `url` it was constructed with: |
| 12 | + * |
| 13 | + * | filter | LOCAL (this compiler) | REMOTE (`RemoteTransport`) | |
| 14 | + * |---|---|---| |
| 15 | + * | `{ d: undefined }` | knex `Undefined binding(s)` | `['3','4']` | |
| 16 | + * | `{ d: { $eq: undefined } }` | knex `Undefined binding(s)` | `['3','4']` | |
| 17 | + * | `{ $not: { d: undefined } }` | knex `Undefined binding(s)` | `['1','2']` | |
| 18 | + * | `{ d: { $ne: undefined } }` | `['1','2']` | `['1','2']` | |
| 19 | + * | `{ $not: { d: { $ne: undefined } } }` | `[]` | `['3','4']` | |
| 20 | + * | `{ d: { $in: [undefined] } }` | knex `Undefined binding(s)` | `[]` | |
| 21 | + * | `{ d: { $gt: undefined } }` | knex `Undefined binding(s)` | `[]` | |
| 22 | + * |
| 23 | + * Two defects, both closed by ONE gate placed before any emitter or guard runs: |
| 24 | + * |
| 25 | + * **A — no ADR-0112 envelope.** knex's `Undefined binding(s) detected when |
| 26 | + * compiling SELECT` carries neither `code` nor `status`, so `mapDataError` fell |
| 27 | + * to its default branch and served an opaque 500 for what is a caller mistake in |
| 28 | + * a filter. #1116 / #4436 catalogued this exact shape for other inputs; this one |
| 29 | + * was missing from the list. |
| 30 | + * |
| 31 | + * **B — the guard disagreed with its own emitter.** The `$ne` emitter read |
| 32 | + * `coerced == null` (LOOSE — `undefined` compiled `IS NOT NULL`, a TOTAL |
| 33 | + * predicate) while `operatorIsNullTotal` / `nullValueSatisfiesOperator` read |
| 34 | + * `=== null` (STRICT — they judged the same leaf non-total AND satisfied by a |
| 35 | + * NULL row). `nullGuardForFieldSpec` therefore wrapped a total predicate in |
| 36 | + * `d IS NULL OR d IS NOT NULL` — a tautology whose negation is FALSE, which is |
| 37 | + * the `[]` above. #5298's invariant is that a polarity table pins the spelling |
| 38 | + * of ITS OWN emitter; this was that invariant broken at its own definition. |
| 39 | + * |
| 40 | + * # The ruling |
| 41 | + * |
| 42 | + * REFUSED (`INVALID_FILTER` / 400), adjudicated 2026-08-07 on #6050 — the |
| 43 | + * disposition #5347-A gave a non-boolean `$null`. `FieldOperatorsSchema` |
| 44 | + * declares no `undefined` comparand; `{ f: undefined }` and `{}` are the same |
| 45 | + * object to every JavaScript reader while meaning opposite things (a predicate |
| 46 | + * versus no constraint at all); and `undefined` cannot survive a JSON round |
| 47 | + * trip, so it is always the fingerprint of an in-process authoring bug — the |
| 48 | + * `{ owner_id: ctx.user?.id }` that silently matched every env-wide row. |
| 49 | + * |
| 50 | + * # Reverse verification — direction predicted before it was run, then measured |
| 51 | + * |
| 52 | + * Prediction: NOT one uniform direction. The refusal cases must go red, the |
| 53 | + * control cases must stay green, and the refusal cases must go red by TWO |
| 54 | + * different mechanisms — because the un-fixed driver answered this family two |
| 55 | + * different ways (see the matrix above). |
| 56 | + * |
| 57 | + * Measured, with both `assertDefinedComparands` call sites deleted and nothing |
| 58 | + * else changed: **22 failed / 6 passed** of 28. The 6 that stayed green are |
| 59 | + * exactly the control cases — the `null` block, the `$null`/`$exists` |
| 60 | + * boolean-domain case, the legal-vocabulary case and the `Date` case — which is |
| 61 | + * the assertion that this change moved nothing it was not ruled to move. |
| 62 | + * |
| 63 | + * The two mechanisms, per the `origin/main` measurement above: most positions |
| 64 | + * went red by THROWING knex's bare `Undefined binding(s)` — an Error with no |
| 65 | + * `code` and no `status`, so `refusalOf` returns normally and only the envelope |
| 66 | + * assertions fail. **A test that asserted merely "it throws" would have stayed |
| 67 | + * GREEN on the driver this issue was filed against**, which is why every case |
| 68 | + * here asserts `code` and `status`. The rest — `$ne`, the LIKE family, and |
| 69 | + * `{ $not: { … $ne: undefined } }` — went red by RESOLVING: they never threw at |
| 70 | + * all, and `{ $not: { stage: { $ne: undefined } } }` resolved to `[]`, the |
| 71 | + * defect-B tautology. |
| 72 | + */ |
| 73 | + |
| 74 | +import { describe, it, expect, beforeEach } from 'vitest'; |
| 75 | +import { SqlDriver } from './index.js'; |
| 76 | +import type { FilterCondition } from '@objectstack/spec/data'; |
| 77 | + |
| 78 | +interface WireBearingError extends Error { |
| 79 | + code?: string; |
| 80 | + status?: number; |
| 81 | +} |
| 82 | + |
| 83 | +describe('[#6050] SqlDriver refuses an undefined comparand', () => { |
| 84 | + let driver: SqlDriver; |
| 85 | + |
| 86 | + beforeEach(async () => { |
| 87 | + driver = new SqlDriver({ |
| 88 | + client: 'better-sqlite3', |
| 89 | + connection: { filename: ':memory:' }, |
| 90 | + useNullAsDefault: true, |
| 91 | + }); |
| 92 | + await driver.initObjects([ |
| 93 | + { |
| 94 | + name: 'deal', |
| 95 | + fields: { |
| 96 | + id: { type: 'text', name: 'id' }, |
| 97 | + stage: { type: 'text', name: 'stage' }, |
| 98 | + score: { type: 'number', name: 'score' }, |
| 99 | + }, |
| 100 | + } as any, |
| 101 | + ]); |
| 102 | + await driver.create('deal', { id: '1', stage: 'won', score: 10 }); |
| 103 | + await driver.create('deal', { id: '2', stage: null, score: 20 }); |
| 104 | + }); |
| 105 | + |
| 106 | + const ids = async (where: unknown): Promise<string[]> => { |
| 107 | + const rows = await driver.find('deal', { |
| 108 | + object: 'deal', |
| 109 | + fields: ['id'], |
| 110 | + where: where as FilterCondition, |
| 111 | + }); |
| 112 | + return (rows as any[]).map((r) => String(r.id)).sort(); |
| 113 | + }; |
| 114 | + |
| 115 | + const refusalOf = async (where: unknown): Promise<WireBearingError> => { |
| 116 | + try { |
| 117 | + await ids(where); |
| 118 | + } catch (e) { |
| 119 | + return e as WireBearingError; |
| 120 | + } |
| 121 | + throw new Error('expected the driver to refuse this filter, but it resolved'); |
| 122 | + }; |
| 123 | + |
| 124 | + /** |
| 125 | + * Every position a comparand can occupy, with the path the refusal must name. |
| 126 | + * |
| 127 | + * "Comparand" is a POSITION, not a type, so the list is enumerated rather than |
| 128 | + * swept: the direct comparand, each single-value operator's comparand, and |
| 129 | + * each MEMBER of a list operator's array — the same split #5041 made when it |
| 130 | + * refused a `$field` reference inside an `$in` list. |
| 131 | + */ |
| 132 | + const UNDEFINED_POSITIONS: Array<[label: string, where: unknown, path: string]> = [ |
| 133 | + // The shape the issue opened on. It is also the one that does NOT reach the |
| 134 | + // reduction walk: `typeof undefined` is not `'object'`, so it cannot make |
| 135 | + // `applyFilters`' `hasMongoOperators` test true and the plain-map loop |
| 136 | + // compiles it instead. Both call sites share one function for this reason. |
| 137 | + ['a direct comparand', { stage: undefined }, 'filter.stage'], |
| 138 | + ['$eq', { stage: { $eq: undefined } }, 'filter.stage.$eq'], |
| 139 | + ['$ne', { stage: { $ne: undefined } }, 'filter.stage.$ne'], |
| 140 | + ['$gt', { score: { $gt: undefined } }, 'filter.score.$gt'], |
| 141 | + ['$gte', { score: { $gte: undefined } }, 'filter.score.$gte'], |
| 142 | + ['$lt', { score: { $lt: undefined } }, 'filter.score.$lt'], |
| 143 | + ['$lte', { score: { $lte: undefined } }, 'filter.score.$lte'], |
| 144 | + ['$contains', { stage: { $contains: undefined } }, 'filter.stage.$contains'], |
| 145 | + ['$notContains', { stage: { $notContains: undefined } }, 'filter.stage.$notContains'], |
| 146 | + ['$startsWith', { stage: { $startsWith: undefined } }, 'filter.stage.$startsWith'], |
| 147 | + ['$endsWith', { stage: { $endsWith: undefined } }, 'filter.stage.$endsWith'], |
| 148 | + // The array IS `$in`'s comparand; each ELEMENT is a comparand in its own |
| 149 | + // right, and one bad element is enough. |
| 150 | + ['an $in member', { stage: { $in: [undefined] } }, 'filter.stage.$in[0]'], |
| 151 | + ['an $in member beside a good one', { stage: { $in: ['won', undefined] } }, 'filter.stage.$in[1]'], |
| 152 | + ['a $nin member', { stage: { $nin: [undefined] } }, 'filter.stage.$nin[0]'], |
| 153 | + ['a $between bound', { score: { $between: [5, undefined] } }, 'filter.score.$between[1]'], |
| 154 | + // Nested positions — the refusal must name where it happened, not just that |
| 155 | + // it happened. |
| 156 | + ['inside $and', { $and: [{ stage: undefined }] }, 'filter.$and[0].stage'], |
| 157 | + ['inside $or', { $or: [{ stage: { $eq: undefined } }] }, 'filter.$or[0].stage.$eq'], |
| 158 | + ['inside $not', { $not: { stage: undefined } }, 'filter.$not.stage'], |
| 159 | + ['inside $not, one operator down', { $not: { stage: { $ne: undefined } } }, 'filter.$not.stage.$ne'], |
| 160 | + ]; |
| 161 | + |
| 162 | + for (const [label, where, path] of UNDEFINED_POSITIONS) { |
| 163 | + it(`refuses ${label} with INVALID_FILTER / 400`, async () => { |
| 164 | + const err = await refusalOf(where); |
| 165 | + // Defect A: the envelope. Without these two lines the test passes on the |
| 166 | + // UNFIXED driver, which threw knex's bare `Undefined binding(s)`. |
| 167 | + expect(err.code).toBe('INVALID_FILTER'); |
| 168 | + expect(err.status).toBe(400); |
| 169 | + expect(err.message).toContain('is undefined'); |
| 170 | + expect(err.message).toContain(path); |
| 171 | + // The repair the author needs, not just the complaint. |
| 172 | + expect(err.message).toContain('Write null if you meant the null predicate'); |
| 173 | + expect(err.message).toContain('FieldOperatorsSchema'); |
| 174 | + // #3867 — no driver-internal prefix on the wire. |
| 175 | + expect(err.message).not.toContain('[sql-driver]'); |
| 176 | + // It must never BE knex's message. Checked on the opening rather than by |
| 177 | + // absence of the phrase, because this refusal deliberately QUOTES knex's |
| 178 | + // wording in its explanatory tail — a `not.toContain` here would be a |
| 179 | + // phantom assertion that fails on the fixed driver and passes on nothing. |
| 180 | + expect(err.message.startsWith('Comparand at ')).toBe(true); |
| 181 | + }); |
| 182 | + } |
| 183 | + |
| 184 | + /** |
| 185 | + * The placement proof, in the same shape #5348 used. `{ stage: 'won' }` is a |
| 186 | + * satisfiable disjunct and `{}` is the TRUE identity — an emitter-side gate |
| 187 | + * would resolve the enclosing node from those siblings and never look at the |
| 188 | + * offending one, making the refusal conditional on evaluation order. |
| 189 | + */ |
| 190 | + it('refuses an undefined comparand beside a satisfiable disjunct', async () => { |
| 191 | + const err = await refusalOf({ $or: [{ stage: 'won' }, { stage: undefined }] }); |
| 192 | + expect(err.code).toBe('INVALID_FILTER'); |
| 193 | + expect(err.message).toContain('filter.$or[1].stage'); |
| 194 | + }); |
| 195 | + |
| 196 | + it('refuses an undefined comparand beside the TRUE identity `{}`', async () => { |
| 197 | + const err = await refusalOf({ $or: [{}, { stage: { $eq: undefined } }] }); |
| 198 | + expect(err.code).toBe('INVALID_FILTER'); |
| 199 | + expect(err.message).toContain('filter.$or[1].stage.$eq'); |
| 200 | + }); |
| 201 | + |
| 202 | + /** |
| 203 | + * Defect B, pinned as a REFUSAL rather than as an answer. |
| 204 | + * |
| 205 | + * This is the row that answered `[]` locally and `['3','4']` remotely, and the |
| 206 | + * one whose cause was internal to this file: guard `=== null`, emitter |
| 207 | + * `== null`, one leaf, two readings. There is no "correct" row set to assert |
| 208 | + * for it any more — the ruling removed the question — so what is pinned is |
| 209 | + * that it is refused, and that the refusal names the `$ne` comparand rather |
| 210 | + * than some downstream consequence of the tautology. |
| 211 | + */ |
| 212 | + it('refuses the guard/emitter split case instead of answering it', async () => { |
| 213 | + const err = await refusalOf({ $not: { stage: { $ne: undefined } } }); |
| 214 | + expect(err.code).toBe('INVALID_FILTER'); |
| 215 | + expect(err.status).toBe(400); |
| 216 | + expect(err.message).toContain('filter.$not.stage.$ne'); |
| 217 | + }); |
| 218 | + |
| 219 | + /** |
| 220 | + * ⛔ The control block: `null` did not move, in any position. |
| 221 | + * |
| 222 | + * A gate written by pattern-matching on "empty-ish comparand" would take |
| 223 | + * `null` with it and silently delete the null predicate from the protocol. |
| 224 | + * Every line here answered exactly this before the change. |
| 225 | + */ |
| 226 | + describe('null comparands are unchanged, line by line', () => { |
| 227 | + it('the null predicate spellings still compile', async () => { |
| 228 | + expect(await ids({ stage: null })).toEqual(['2']); |
| 229 | + expect(await ids({ stage: { $eq: null } })).toEqual(['2']); |
| 230 | + expect(await ids({ stage: { $ne: null } })).toEqual(['1']); |
| 231 | + expect(await ids({ stage: { $null: true } })).toEqual(['2']); |
| 232 | + expect(await ids({ stage: { $null: false } })).toEqual(['1']); |
| 233 | + expect(await ids({ stage: { $exists: true } })).toEqual(['1']); |
| 234 | + expect(await ids({ stage: { $exists: false } })).toEqual(['2']); |
| 235 | + }); |
| 236 | + |
| 237 | + it('null keeps its meaning under $not and inside combinators', async () => { |
| 238 | + expect(await ids({ $not: { stage: null } })).toEqual(['1']); |
| 239 | + expect(await ids({ $not: { stage: { $ne: null } } })).toEqual(['2']); |
| 240 | + expect(await ids({ $and: [{ stage: null }] })).toEqual(['2']); |
| 241 | + expect(await ids({ $or: [{ stage: null }, { stage: 'won' }] })).toEqual(['1', '2']); |
| 242 | + }); |
| 243 | + |
| 244 | + it('null is still a legitimate $in / $nin member', async () => { |
| 245 | + // `IN (NULL)` is UNKNOWN for every row in SQL — that is SQL's answer, not |
| 246 | + // this gate's, and it is untouched here. |
| 247 | + expect(await ids({ stage: { $in: ['won', null] } })).toEqual(['1']); |
| 248 | + }); |
| 249 | + }); |
| 250 | + |
| 251 | + /** |
| 252 | + * `$null` / `$exists` keep their OWN refusal for `undefined`. |
| 253 | + * |
| 254 | + * Their comparand is a declared BOOLEAN — a flag, not a value to compare |
| 255 | + * against — and `nonBooleanNullComparandError` / `nonBooleanExistsComparandError` |
| 256 | + * already name that declared domain, which is the more useful message for |
| 257 | + * that mistake. #5240's rule is one condition, one wording; re-answering these |
| 258 | + * two here would have given the same mistake two. |
| 259 | + */ |
| 260 | + it('$null / $exists undefined keeps the boolean-domain wording', async () => { |
| 261 | + for (const where of [{ stage: { $null: undefined } }, { stage: { $exists: undefined } }]) { |
| 262 | + const err = await refusalOf(where); |
| 263 | + expect(err.code).toBe('INVALID_FILTER'); |
| 264 | + expect(err.message).toContain('requires a boolean comparand (true or false)'); |
| 265 | + expect(err.message).not.toContain('Write null if you meant the null predicate'); |
| 266 | + } |
| 267 | + }); |
| 268 | + |
| 269 | + /** |
| 270 | + * The ordinary vocabulary is untouched. A gate on the reduction walk runs for |
| 271 | + * every filter in the process, so "did it change an answer it should not have" |
| 272 | + * is a question this file has to ask out loud. |
| 273 | + */ |
| 274 | + it('legal filters compile exactly as before', async () => { |
| 275 | + expect(await ids({ stage: 'won' })).toEqual(['1']); |
| 276 | + expect(await ids({ stage: { $in: ['won'] } })).toEqual(['1']); |
| 277 | + expect(await ids({ stage: { $ne: 'won' } })).toEqual(['2']); |
| 278 | + expect(await ids({ score: { $between: [5, 15] } })).toEqual(['1']); |
| 279 | + expect(await ids({ score: { $gte: 10 } })).toEqual(['1', '2']); |
| 280 | + expect(await ids({ $not: { stage: 'won' } })).toEqual(['2']); |
| 281 | + expect(await ids({})).toEqual(['1', '2']); |
| 282 | + expect(await ids({ $and: [] })).toEqual(['1', '2']); |
| 283 | + expect(await ids({ $or: [] })).toEqual([]); |
| 284 | + }); |
| 285 | + |
| 286 | + /** |
| 287 | + * A Date comparand is an OBJECT that is a VALUE. The walk must not read its |
| 288 | + * (empty) entry list as an operator map and must not refuse it — the #1066 |
| 289 | + * routing seam, asserted from the gate's side. |
| 290 | + */ |
| 291 | + it('a Date comparand is not mistaken for an operator map', async () => { |
| 292 | + // No row matches; the point is that it COMPILES rather than being refused. |
| 293 | + expect(await ids({ stage: { $ne: new Date('2020-01-01T00:00:00.000Z') } })).toEqual(['1', '2']); |
| 294 | + }); |
| 295 | +}); |
0 commit comments