From 936a5b5f399f6f4ab5f3aacd727f813c1fbfb987 Mon Sep 17 00:00:00 2001 From: David de Boer Date: Thu, 20 Aug 2026 17:27:23 +0200 Subject: [PATCH] fix(search-typesense)!: compile a where clause that can match nothing to a term matching nothing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A clause whose every criterion was false compiled to no term, and a clause with no term was left out of filter_by. An absent conjunct constrains nothing, so false was compiled as true: a single misspelled field name in where turned a filtered search into the whole collection. - a clause with criteria but no terms now compiles to id:=[] – an empty identity membership, the same reading isUnsatisfiable gives one in the IR, and a term Typesense applies rather than ignores - a clause carrying no criteria at all still states nothing, not false, so it stays the vacuous no-op it reads as - onIgnoredFilter keeps reporting both, since neither compiles as written Not reachable through search(): assertValidQuery rejects an unknown field, a non-filterable one and an operator mismatch first, and isUnsatisfiable answers the empty id membership without a round-trip. It is reachable for every direct caller of buildSearchParams. BREAKING CHANGE: buildSearchParams compiles a where clause it cannot compile to “id:=[]” instead of omitting it, so a query that used to come back with every document now comes back empty. onIgnoredFilter still fires for it. --- docs/reference/search.md | 24 +++++-- .../search-typesense/src/query-compiler.ts | 64 +++++++++++++------ packages/search-typesense/test/joins.test.ts | 6 +- .../test/query-compiler.test.ts | 44 +++++++++---- .../test/search-engine.test.ts | 28 ++++++++ 5 files changed, 128 insertions(+), 38 deletions(-) diff --git a/docs/reference/search.md b/docs/reference/search.md index 31d3a777..d8cfd5e3 100644 --- a/docs/reference/search.md +++ b/docs/reference/search.md @@ -707,12 +707,24 @@ One rule is about a **value** rather than a field: a `date` range bound the storage codec cannot read (`'yesterday'`, or a year past the ±271,821 window `Date` covers) is an `unparseable-bound` issue naming the rejected bound. No surface’s type system catches this – `String` is `String` – and it has to be -caught here because **no compiler can recover from it**: an engine filter -language has no term meaning “matches nothing”, so a criterion it cannot compile -either states no constraint (widening the search to everything) or vanishes from -the query (widening it to whatever the other clauses allow). Both answer a -question the caller did not ask, and only the caller can fix the bound – so -validation hands it back to them instead of guessing. +caught here because a compiler cannot recover what the caller meant: a rejected +bound is indistinguishable from a bound never set, and inside an `or` the two +readings pull opposite ways – read as unset it widens the range to unbounded, +dropped it narrows the result to the criterion’s siblings. Only the caller can +fix the bound, so validation hands it back to them instead of guessing. + +Validation is the guard, but it is not the only one: a compiler reached +directly must not quietly answer a different question either. So a `where` +clause the Typesense compiler cannot compile is treated by **what it means**, +not by the fact that it produced no term. A clause that states no constraint +(an empty `in`, a `range` with no usable bound) is _true_ and leaves the query; +a clause whose every criterion is malformed or unsatisfiable is _false_ and +compiles to a term no document matches, so the search comes back empty. The +difference only shows up in the `&&` between clauses – dropping a false clause +would delete a conjunct and hand back everything the remaining clauses allow – +and it is why a filter language needs a way to say “nothing”, which `filter_by` +spells as the empty identity membership `id:=[]`. Either way the clause is +reported to `onIgnoredFilter`, since neither compiled as written. ### Lookup by IRI diff --git a/packages/search-typesense/src/query-compiler.ts b/packages/search-typesense/src/query-compiler.ts index 4343e11b..86ddbca4 100644 --- a/packages/search-typesense/src/query-compiler.ts +++ b/packages/search-typesense/src/query-compiler.ts @@ -65,12 +65,14 @@ export interface BuildSearchParamsOptions { */ readonly maxFacetValues?: number; /** - * Called for each `where` clause that compiles to nothing and is therefore - * skipped: an unknown field, an operator that does not match the field’s - * kind ({@link filterOperatorFor}), an empty `in` list, or a `range` with no - * usable bound. Skipping keeps a malformed clause from reaching the engine - * as garbage; supply this to log it instead of losing it silently. Through - * the engine, a structurally invalid query throws up front + * Called for each `where` clause that does not compile as the caller wrote + * it – either because it states no constraint (an empty `in` list, a `range` + * with no usable bound, no criteria at all) and is skipped, or because every + * criterion is malformed (an unknown field, an operator that does not match + * the field’s kind – {@link filterOperatorFor}) or unsatisfiable, leaving a + * clause no document can match. Supply this to log the clause instead of + * losing it silently; the compiled query itself is faithful either way. + * Through the engine, a structurally invalid query throws up front * (`assertValidQuery`), so there only the clauses that state no constraint * reach this. */ @@ -196,8 +198,10 @@ function queryFields( return { names, weights }; } -/** AND-join the compiled `where` clauses; a clause that compiles to nothing is - * skipped and reported to `onIgnoredFilter`. */ +/** AND-join the compiled `where` clauses. A clause that states no constraint is + * skipped; one that can match nothing compiles to {@link MATCHES_NOTHING} + * rather than being skipped ({@link compileFilter}). Either way it did not + * compile as written, so it is reported to `onIgnoredFilter`. */ function compileFilterBy( where: readonly Filter[], searchType: SearchType, @@ -206,7 +210,7 @@ function compileFilterBy( return where .map((filter) => { const clause = compileFilter(filter, searchType, options); - if (clause === undefined) { + if (clause === undefined || clause === MATCHES_NOTHING) { options.onIgnoredFilter?.(filter); } return clause; @@ -235,15 +239,16 @@ function compileFilterBy( * unknown field, an operator that mismatches the field’s kind) is *false*, and * `false || X` is X, so it drops out and its siblings still stand. * - * Both readings hold **within** a clause. Neither survives a clause left with - * no terms: it is skipped and reported to `onIgnoredFilter`, and a clause - * missing from the conjunction constrains nothing – so a *false* criterion - * alone in its clause still WIDENS the query. `filter_by` has no term meaning - * “matches nothing” to compile it to instead. The engine never meets this - * (`assertValidQuery` rejects every malformed criterion, and `isUnsatisfiable` - * short-circuits the empty `id` membership before a query is dispatched); a - * direct caller should read `onIgnoredFilter` firing as “this query no longer - * asks what you asked”. + * Which reading applies decides what a clause left with **no terms** compiles + * to, and the two are opposites: a vacuous clause states no constraint, so it + * is skipped; an all-false clause states the query can have no answer, so it + * compiles to {@link MATCHES_NOTHING}. Skipping that one instead would drop a + * conjunct and widen the query – `false` compiled as `true`. Both are reported + * to `onIgnoredFilter`, because neither compiles to what the caller wrote. + * + * The engine never meets either: `assertValidQuery` rejects every malformed + * criterion, and `isUnsatisfiable` short-circuits the empty `id` membership + * before a query is dispatched. */ function compileFilter( filter: Filter, @@ -261,11 +266,32 @@ function compileFilter( } } if (terms.length === 0) { - return undefined; + // Every criterion was unusable, so the clause is FALSE – and `false && X` + // is false, however many clauses stand beside it. Leaving it out would drop + // a conjunct, and a conjunct missing from `filter_by` constrains nothing: + // the query would come back WIDER than the caller wrote, which is how a + // misspelled field name used to return the whole collection. + // + // A clause carrying no criteria at all says nothing rather than saying + // false, so it stays the vacuous no-op it reads as. + return filter.or.length === 0 ? undefined : MATCHES_NOTHING; } return terms.length === 1 ? terms[0] : `(${terms.join(' || ')})`; } +/** + * The term for a clause that can match no document: an **empty identity + * membership** – “the document is one of no documents”. Not a sentinel value + * but the literal reading, and the same one {@link isUnsatisfiable} gives an + * empty `id` membership in the IR; Typesense answers it with zero hits. + * + * A filter language has no keyword for `false`, so this stands in for one. It + * must be a term the engine *applies* rather than ignores – an empty string + * (``id:=` ` ``) is rejected outright as a filter value, and an omitted clause + * is read as true. + */ +const MATCHES_NOTHING = `${ID_FIELD}:=[]`; + /** A criterion that states **no constraint** – true for every document. */ const VACUOUS = Symbol('vacuous'); /** A criterion that matches nothing, or cannot be compiled at all – false. */ diff --git a/packages/search-typesense/test/joins.test.ts b/packages/search-typesense/test/joins.test.ts index 8f0447d3..449dff28 100644 --- a/packages/search-typesense/test/joins.test.ts +++ b/packages/search-typesense/test/joins.test.ts @@ -199,7 +199,9 @@ describe('buildSearchParams over a join path', () => { ], }), ).toBe('country:[`BE`]'); - // And with no resolver supplied at all. + // And with no resolver supplied at all: the criterion is alone in its + // clause, so there is no sibling to fall back to and the clause is simply + // false – which must EMPTY the query, not disappear from it. expect( buildSearchParams( { @@ -208,7 +210,7 @@ describe('buildSearchParams over a join path', () => { }, CREATIVE_WORK, ).filter_by, - ).toBeUndefined(); + ).toBe('id:=[]'); }); }); diff --git a/packages/search-typesense/test/query-compiler.test.ts b/packages/search-typesense/test/query-compiler.test.ts index cbcc475d..feaed297 100644 --- a/packages/search-typesense/test/query-compiler.test.ts +++ b/packages/search-typesense/test/query-compiler.test.ts @@ -266,7 +266,10 @@ describe('buildSearchParams', () => { ).toBeUndefined(); }); - it('ignores a clause when no criterion compiles', () => { + it('compiles a clause no criterion of which compiles to a term matching nothing', () => { + // Every criterion is false, so the clause is false and the query can have + // no answer. Leaving it out of `filter_by` would say the opposite – an + // absent conjunct constrains nothing – and hand back the whole collection. const ignored: unknown[] = []; const clause = { or: [ @@ -277,7 +280,8 @@ describe('buildSearchParams', () => { const params = buildSearchParams({ ...base, where: [clause] }, schema, { onIgnoredFilter: (filter) => ignored.push(filter), }); - expect(params.filter_by).toBeUndefined(); + expect(params.filter_by).toBe('id:=[]'); + // Still reported: the clause did not compile as written. expect(ignored).toEqual([clause]); }); @@ -300,7 +304,12 @@ describe('buildSearchParams', () => { ); }); - it('skips a vacuous or non-membership `id` clause', () => { + it('reads an empty or non-membership `id` clause as matching no document', () => { + // An empty `id` membership enumerates NO document, and a range on an IRI + // is malformed – both false, so each clause compiles to the term nothing + // satisfies. `search()` never sends this: `isUnsatisfiable` answers the + // first shape without a round-trip and `assertValidQuery` rejects the + // second. const ignored: unknown[] = []; const params = buildSearchParams( { @@ -313,30 +322,34 @@ describe('buildSearchParams', () => { schema, { onIgnoredFilter: (filter) => ignored.push(filter) }, ); - expect(params.filter_by).toBeUndefined(); + expect(params.filter_by).toBe('id:=[] && id:=[]'); expect(ignored).toEqual([ { or: [{ field: 'id', in: [] }] }, { or: [{ field: 'id', range: { min: 1 } }] }, ]); }); - it('skips a clause that compiles to nothing and reports it via onIgnoredFilter', () => { + it('separates a clause that says nothing from one that can match nothing', () => { + // The two ways a clause fails to compile as written are opposites, and the + // `&&` between clauses is what makes the difference visible: an unset + // filter must LEAVE the query, while a false one must stay and empty it. + // Both are reported, because neither says what the caller wrote. const ignored: unknown[] = []; const params = buildSearchParams( { ...base, where: [ { or: [{ field: 'status', in: ['valid'] }] }, // fine – kept - { or: [{ field: 'nonexistent', in: ['x'] }] }, // unknown field - { or: [{ field: 'keyword', range: { min: 1 } }] }, // operator ≠ field kind - { or: [{ field: 'status', in: [] }] }, // empty membership - { or: [{ field: 'size', range: {} }] }, // no usable bound + { or: [{ field: 'nonexistent', in: ['x'] }] }, // unknown field – false + { or: [{ field: 'keyword', range: { min: 1 } }] }, // operator ≠ kind – false + { or: [{ field: 'status', in: [] }] }, // empty membership – no constraint + { or: [{ field: 'size', range: {} }] }, // no usable bound – no constraint ], }, schema, { onIgnoredFilter: (filter) => ignored.push(filter) }, ); - expect(params.filter_by).toBe('status:[`valid`]'); + expect(params.filter_by).toBe('status:[`valid`] && id:=[] && id:=[]'); expect(ignored).toEqual([ { or: [{ field: 'nonexistent', in: ['x'] }] }, { or: [{ field: 'keyword', range: { min: 1 } }] }, @@ -345,11 +358,20 @@ describe('buildSearchParams', () => { ]); }); - it('skips a non-compiling clause silently when no onIgnoredFilter is given', () => { + it('compiles a false clause the same way when no onIgnoredFilter is given', () => { + // The callback is diagnostics, not control flow: what reaches the engine + // must not depend on whether anyone is listening. const params = buildSearchParams( { ...base, where: [{ or: [{ field: 'nonexistent', in: ['x'] }] }] }, schema, ); + expect(params.filter_by).toBe('id:=[]'); + }); + + it('skips a clause carrying no criteria at all', () => { + // `{ or: [] }` states nothing rather than stating false – there is no + // criterion to be false – so it stays the vacuous no-op it reads as. + const params = buildSearchParams({ ...base, where: [{ or: [] }] }, schema); expect(params.filter_by).toBeUndefined(); }); diff --git a/packages/search-typesense/test/search-engine.test.ts b/packages/search-typesense/test/search-engine.test.ts index 2d3d8d6c..121531f9 100644 --- a/packages/search-typesense/test/search-engine.test.ts +++ b/packages/search-typesense/test/search-engine.test.ts @@ -9,6 +9,7 @@ import { import { describeSearchEngineContract } from '@lde/search/testing'; import { buildCollectionDefinition } from '../src/collection-definition.js'; import { createTypesenseSearchEngine } from '../src/search.js'; +import { buildSearchParams } from '../src/query-compiler.js'; import { TypesenseContainer } from './typesense-container.js'; // The label source `publisher` resolves against: a first-class search type @@ -473,4 +474,31 @@ describe('createTypesenseSearchEngine (integration)', () => { expect(result.total).toBeGreaterThan(0); // empty membership = no constraint expect(ignored).toEqual([{ or: [{ field: 'status', in: [] }] }]); }); + + it('answers a false clause with no hits, against the real engine', async () => { + // The compiler’s term for “matches nothing” only works if Typesense + // APPLIES it: a filter it rejected would fail the search, and one it + // ignored would hand back the whole collection – the bug this replaced. + // `search()` never builds this itself (`assertValidQuery` rejects an + // unknown field first), so the params go to the engine directly. + const everything = await client + .collections('datasets') + .documents() + .search({ q: '*', query_by: 'title_search_nl' }); + expect(everything.found).toBeGreaterThan(0); + + const params = buildSearchParams( + { + ...baseQuery, + where: [{ or: [{ field: 'nonexistent', in: ['x'] }] }], + }, + datasetSchema, + ); + expect(params.filter_by).toBe('id:=[]'); + const none = await client + .collections('datasets') + .documents() + .search({ ...params, filter_by: params.filter_by }); + expect(none.found).toBe(0); + }); });