diff --git a/benchmarks/hnsw-search.js b/benchmarks/hnsw-search.js index 0b416aec8d..7fa0177f94 100644 --- a/benchmarks/hnsw-search.js +++ b/benchmarks/hnsw-search.js @@ -223,7 +223,7 @@ for (const p of SELECTIVITIES) { // predicate-aware traversal const t0 = performance.now(); - const pred = hnsw.search({ target: query, comparator: 'sort', descending: false }, {}, filter); + const pred = hnsw.search({ target: query, comparator: 'sort', descending: false }, {}, { filter }); predNs += (performance.now() - t0) * 1e6; const predIdx = new Set(pred.slice(0, k).map((r) => idxOf(r.key))); predRecall += overlap(gt, predIdx) / k; diff --git a/resources/DESIGN.md b/resources/DESIGN.md index fffde552d6..ed46fd9f43 100644 --- a/resources/DESIGN.md +++ b/resources/DESIGN.md @@ -93,7 +93,7 @@ One giant `makeTable()` factory that returns a `TableResource extends Resource` | How does a query opt out of a read snapshot? | Pass `snapshot: false` on the search request (e.g. `get_analytics`). `Table.ts → search` calls `txn.useReadTxn(snapshot === false)`; on RocksDB `DatabaseTransaction.getReadTxn` then builds the read txn with `{ disableSnapshot: true }` so a long scan reads latest without pinning a snapshot. No-op on LMDB (`LMDBTransaction.useReadTxn`). | | How does a URL path map to a Resource? | `Resources.ts → getMatch` (exact/prefix fast path) then `matchParamRoute` (parameterised routes); see "Path routing" below | | How does HNSW keep the graph connected on delete? | `indexes/HierarchicalNavigableSmallWorld.ts → index()` delete path: zero-degree orphans reindexed via `needsReindexing`; severed multi-node islands detected and reconnected by `repairSeveredNeighbors` (#1712) | -| How is a filter applied _during_ a vector search? | Predicate-aware traversal (#1241): `search.ts → executeConditions` composes companion AND conditions with request `vectorFilter` / `rowFilter` predicates into one `(primaryKey) => boolean` (`composeRecordFilter`) and passes it to `HierarchicalNavigableSmallWorld.search(cond, ctx, filter)`. The filter gates result admission at layer 0 only (routing ignores it, ACORN-style); a visit budget (`filterExpansion`) bounds the under-filled/selective case. Very selective _condition_ filters are instead diverted to the exact brute-force path by the query planner's `estimateCountAsSort` ordering. | +| How is a filter applied _during_ a vector search? | Predicate-aware traversal (#1241): `search.ts → executeConditions` composes companion AND conditions with request `vectorFilter` / `rowFilter` predicates into one `(primaryKey) => boolean` (`composeRecordFilter`) and passes it to `HierarchicalNavigableSmallWorld.search(cond, ctx, { filter })`. The filter gates result admission at layer 0 only (routing ignores it, ACORN-style); a visit budget (`filterExpansion`) bounds the under-filled/selective case. Very selective _condition_ filters are instead diverted to the exact brute-force path by the query planner's `estimateCountAsSort` ordering. | | How does post-ordering resolve vector distances safely? | Each comparator owns its `Sort`, passes it directly to the custom-index resolver, and caches distances by that immutable per-query sort object. | | How is application row filtering applied? | Authorization admission happens in the resource operation before query work. The legacy `allow*` hook, when armed by the protocol, is evaluated once with its historical receiver semantics; overriding it never changes its scope. An operation override may add indexed conditions and/or attach the JavaScript-only synchronous `target.rowFilter(record, context)`. `Table.search` composes it with query filters and rechecks the final materialized cache/source record. `SubscriptionRequest.rowFilter` covers full-row events; `eventFilter(event, context)` explicitly handles tombstones/messages/raw events. Prefer indexed conditions because an opaque predicate may inspect every admitted candidate and `limit` applies after filtering. | diff --git a/resources/Table.ts b/resources/Table.ts index c96c7e2849..59063a1b84 100644 --- a/resources/Table.ts +++ b/resources/Table.ts @@ -4962,7 +4962,7 @@ export function makeTable(options) { txnForContext(context).getReadTxn(), false, relatedTable, - false + { allowFullScan: false } ) as any ).map((entry) => { if (entry && entry.key !== undefined) return entry; diff --git a/resources/indexes/HierarchicalNavigableSmallWorld.ts b/resources/indexes/HierarchicalNavigableSmallWorld.ts index f028ff07df..4c12f45fac 100644 --- a/resources/indexes/HierarchicalNavigableSmallWorld.ts +++ b/resources/indexes/HierarchicalNavigableSmallWorld.ts @@ -961,12 +961,17 @@ export class HierarchicalNavigableSmallWorld { * This the main entry from Harper's query functionality, where we actually search for an ordered list of nearest * neighbors, using the provided sort/order definition object and performing the multi-layer skip-list search. * This returns an iterable of the nearest neighbors to the provided target vector, with nearest ordered first. + * + * This is also the contract an index implemented outside this repo has to satisfy, so everything + * optional is named: a future capability (a paging cursor, a deadline, a recall target) is a new + * field on `options` rather than a positional argument that breaks every existing implementation. * @param target * @param value * @param descending * @param distance * @param comparator * @param context + * @param options */ search( { @@ -987,16 +992,21 @@ export class HierarchicalNavigableSmallWorld { filterExpansion?: number; }, context: any, - // Predicate-aware traversal (#1241). When provided, only nodes for which `filter(primaryKey)` - // returns true are admitted to the result list at layer 0; routing is unaffected. Composed by - // search.ts from companion AND conditions and caller-supplied vector/row filters. Must be - // synchronous and side-effect free. JS-API only (never from a REST query string). - filter?: (primaryKey: Id) => boolean, - // offset + limit for a bounded query. A layer-0 search returns at most `ef` candidates, so a - // query asking for more rows than that used to come back short with no error — capped at 512 - // (AUTO_EF_MAX) however large the limit was. Raising ef to cover the request keeps `limit` - // meaningful; the caller pays for what it asked for. - minResults?: number + { + filter, + minResults, + }: { + // Predicate-aware traversal (#1241). When provided, only nodes for which `filter(primaryKey)` + // returns true are admitted to the result list at layer 0; routing is unaffected. Composed by + // search.ts from companion AND conditions and caller-supplied vector/row filters. Must be + // synchronous and side-effect free. JS-API only (never from a REST query string). + filter?: (primaryKey: Id) => boolean; + // offset + limit for a bounded query. A layer-0 search returns at most `ef` candidates, so a + // query asking for more rows than that used to come back short with no error — capped at 512 + // (AUTO_EF_MAX) however large the limit was. Raising ef to cover the request keeps `limit` + // meaningful; the caller pays for what it asked for. + minResults?: number; + } = {} ) { let limit: number | undefined; // only set for threshold comparators; 0 is a valid threshold (e.g. dotProduct) let limitInclusive = false; // true for `le`, false for `lt` diff --git a/resources/search.ts b/resources/search.ts index c1f7dcafdf..dcda46cf61 100644 --- a/resources/search.ts +++ b/resources/search.ts @@ -140,16 +140,12 @@ export function executeConditions( filtered // recordAccess intentionally omitted: guards run once, at the top level (see above). ); - return searchByIndex( - condition, - txn, - condition.descending || request.reverse === true, - table, - request.allowFullScan, + return searchByIndex(condition, txn, condition.descending || request.reverse === true, table, { + allowFullScan: request.allowFullScan, filtered, context, - request.limit !== undefined ? (request.offset || 0) + request.limit : undefined - ); + minResults: request.limit !== undefined ? (request.offset || 0) + request.limit : undefined, + }); } function mapConditionsToFilters(conditions, intersection, estimatedIncomingCount) { return conditions @@ -233,26 +229,34 @@ function composeRecordFilter(recordFilters, table, context): (primaryKey: Id) => } /** - * Search for records or keys, based on the search condition, using an index if available + * Search for records or keys, based on the search condition, using an index if available. The four + * leading parameters are required at every call site; everything optional is named, so a new + * capability can be added without shifting positions at callers that don't use it. * @param searchCondition * @param transaction * @param reverse * @param Table - * @param allowFullScan - * @param filtered + * @param options */ export function searchByIndex( searchCondition: DirectCondition, transaction: any, reverse: boolean, Table: any, - allowFullScan?: boolean, - filtered?: any, - context?: any, - // How many rows the query will ultimately consume (offset + limit), when it is bounded. An - // approximate index returns a fixed-size candidate list, so without this a query asking for more - // rows than that list holds silently gets a short result set. Only custom indexes read it. - minResults?: number + { + allowFullScan, + filtered, + context, + minResults, + }: { + allowFullScan?: boolean; + filtered?: any; + context?: any; + // How many rows the query will ultimately consume (offset + limit), when it is bounded. An + // approximate index returns a fixed-size candidate list, so without this a query asking for more + // rows than that list holds silently gets a short result set. Only custom indexes read it. + minResults?: number; + } = {} ): AsyncIterable { let attribute_name = searchCondition[0] ?? searchCondition.attribute; let value = searchCondition[1] ?? searchCondition.value; @@ -284,8 +288,7 @@ export function searchByIndex( transaction, reverse, relatedTable, - allowFullScan, - joined + { allowFullScan, filtered: joined } ); if (attribute.relationship.to) { // this is one-to-many or many-to-many, so we need to track the filtering of related entries that match @@ -302,8 +305,7 @@ export function searchByIndex( transaction, reverse, Table, - allowFullScan, - joined + { allowFullScan, filtered: joined } ); }; if (attribute.elements) { @@ -510,21 +512,23 @@ export function searchByIndex( // exploring until it has enough MATCHING results, rather than post-filtering an under-filled // candidate set. Only indexes that opt in (filteredSearch) receive it; others post-filter as before. const recordFilter = index.customIndex.filteredSearch ? searchCondition.recordFilter : undefined; - const loaded = index.customIndex.search(searchCondition, context, recordFilter, minResults).map((entry) => { - // if the custom index returns an entry with metadata, merge it with the loaded entry - if (typeof entry === 'object' && entry) { - const { key, ...otherProps } = entry; - if (key == null) return SKIP; // primaryKey missing from HNSW node — skip rather than crash - const loadedEntry = Table.primaryStore.getEntry(key, { - transaction: context && Table._readTxnForContext(context), - }); - if (!loadedEntry) return SKIP; // record was deleted/expired or not yet visible - freezeRecord(loadedEntry?.value); - recordRead(loadedEntry); - return { ...otherProps, ...loadedEntry }; - } - return entry; - }); + const loaded = index.customIndex + .search(searchCondition, context, { filter: recordFilter, minResults }) + .map((entry) => { + // if the custom index returns an entry with metadata, merge it with the loaded entry + if (typeof entry === 'object' && entry) { + const { key, ...otherProps } = entry; + if (key == null) return SKIP; // primaryKey missing from HNSW node — skip rather than crash + const loadedEntry = Table.primaryStore.getEntry(key, { + transaction: context && Table._readTxnForContext(context), + }); + if (!loadedEntry) return SKIP; // record was deleted/expired or not yet visible + freezeRecord(loadedEntry?.value); + recordRead(loadedEntry); + return { ...otherProps, ...loadedEntry }; + } + return entry; + }); if (index.customIndex.rescoreResults) { const rescored = index.customIndex.rescoreResults(loaded, searchCondition, comparator, attribute_name); if (rescored != null) return rescored as any; diff --git a/unitTests/resources/searchByIndexOptions.test.js b/unitTests/resources/searchByIndexOptions.test.js new file mode 100644 index 0000000000..e1a97baa26 --- /dev/null +++ b/unitTests/resources/searchByIndexOptions.test.js @@ -0,0 +1,53 @@ +const assert = require('node:assert'); +const { setupTestDBPath } = require('../testUtils'); +const { table } = require('#src/resources/databases'); +const { searchByIndex } = require('#src/resources/search'); +const { setMainIsWorker } = require('#js/server/threads/manageThreads'); + +// searchByIndex takes its optional tail as a named options object (#2165). These assert the options +// are actually read off that object: a caller that reverts to positional arguments would hand +// `false` in where the options belong, and destructuring it yields `allowFullScan === undefined` — +// silently turning a rejected full scan into a permitted one. +describe('searchByIndex options object (#2165)', () => { + if (process.env.HARPER_STORAGE_ENGINE === 'lmdb') return; + let T; + + before(async () => { + setupTestDBPath(); + setMainIsWorker(true); + T = table({ + table: 'SearchByIndexOptionsTest', + database: 'test', + attributes: [ + { name: 'id', isPrimaryKey: true }, + { name: 'indexedName', indexed: true }, + { name: 'unindexedName' }, + ], + }); + await T.put(1, { indexedName: 'a', unindexedName: 'x' }); + await T.put(2, { indexedName: 'b', unindexedName: 'y' }); + }); + + after(() => { + T.dropTable(); + }); + + it('rejects an unindexed attribute when allowFullScan is false in the options object', () => { + assert.throws( + () => + searchByIndex({ attribute: 'unindexedName', value: 'x' }, undefined, false, T, { + allowFullScan: false, + }), + /not indexed/, + 'allowFullScan must be read from the named option' + ); + }); + + it('keeps the permissive defaults when the options object is omitted entirely', async () => { + const ids = []; + for await (const entry of searchByIndex({ attribute: 'unindexedName', value: 'x' }, undefined, false, T)) { + ids.push(entry?.key ?? entry); + } + assert.deepStrictEqual(ids, [1], 'omitting options must full-scan, not inherit allowFullScan: false'); + }); +}); diff --git a/unitTests/resources/vectorIndex.test.js b/unitTests/resources/vectorIndex.test.js index ff029b714a..c7b6f1ca5e 100644 --- a/unitTests/resources/vectorIndex.test.js +++ b/unitTests/resources/vectorIndex.test.js @@ -809,8 +809,7 @@ describe('HNSW limit above the resolved search ef', () => { customIndex.search( { target: [1, 0, 0], comparator: 'sort' }, { transaction: undefined }, - () => true, - 100_000 // a limit far above anything the index would choose for itself + { filter: () => true, minResults: 100_000 } // a limit far above anything the index would choose for itself ); } finally { customIndex.searchLayer = originalSearchLayer; @@ -1959,7 +1958,7 @@ describeUnlessLmdbFilter('HNSW predicate-aware traversal (#1241)', () => { const results = hnsw.search( { target: [0], comparator: 'sort', descending: false }, { transaction: undefined }, - even + { filter: even } ); assert(results.length > 0, 'expected matching results'); assert( @@ -1978,7 +1977,7 @@ describeUnlessLmdbFilter('HNSW predicate-aware traversal (#1241)', () => { const results = hnsw.search( { target: [0], comparator: 'sort', descending: false }, { transaction: undefined }, - everyTenth + { filter: everyTenth } ); assert( results.every((r) => Number(r.key) % 10 === 0), @@ -1998,7 +1997,7 @@ describeUnlessLmdbFilter('HNSW predicate-aware traversal (#1241)', () => { const results = hnsw.search( { target: [0], comparator: 'sort', descending: false, ef: 10, filterExpansion: 2 }, { transaction: undefined }, - matchesNothing + { filter: matchesNothing } ); assert.strictEqual(results.length, 0, 'no matches yields an empty result, not an error'); assert(results.nodesVisited > 0, 'some nodes were visited'); @@ -2011,7 +2010,7 @@ describeUnlessLmdbFilter('HNSW predicate-aware traversal (#1241)', () => { const results = hnsw.search( { target: [0], comparator: 'sort', descending: false }, { transaction: undefined }, - even + { filter: even } ); assert.strictEqual(typeof results.nodesVisited, 'number'); assert.strictEqual(typeof results.filterEvaluations, 'number'); @@ -2027,7 +2026,7 @@ describeUnlessLmdbFilter('HNSW predicate-aware traversal (#1241)', () => { const results = hnsw.search( { target: [0], comparator: 'sort', descending: false }, { transaction: undefined }, - even + { filter: even } ); assert(!results.some((r) => r.key === 10 || r.key === 20), 'deleted nodes must not be returned'); assert( @@ -2042,7 +2041,7 @@ describeUnlessLmdbFilter('HNSW predicate-aware traversal (#1241)', () => { const results = hnsw.search( { target: [0], comparator: 'sort', descending: false }, { transaction: undefined }, - even + { filter: even } ); assert( results.every((r) => Number(r.key) % 2 === 0),