Uh oh!
There was an error while loading. Please reload this page.
fix(vector): a HASH index searched with no condition expression must be refused - #286
Merged
Conversation
…be refused `search_vectors` validated the conditions against the index search schema only when the caller supplied a non-empty `SearchConditionExpression`. An index that declares a HASH element, searched with no expression at all, therefore skipped validation entirely and reached the backend with `hash_key: None`. That contradicted two promises the contract makes. The comment at the resolution site said the index having a HASH element guarantees the search supplied it, and `VectorSearch::hash_key` in `crates/storage/src/lib.rs` tells backend authors that `Some` is always populated when the index declares one, so they may treat it as a mandatory predicate rather than a hint. A backend written against that doc would have served an unscoped search where a scoped one was promised, returning neighbours from every partition instead of the one the caller asked for. The guard is removed, so validation runs unconditionally. With no conditions the membership and type checks iterate nothing and the HASH-presence check does the work, and an index with no search schema still accepts an absent expression. Rather than leave the invariant resting on a comment, the validate-and-resolve step moves into `resolve_search_scope`, which validates, splits the conditions into the partition scope and the inline filters, and asserts that a declared HASH element always yields a populated `hash_key`. A future change that reintroduces a conditional guard fails that assertion instead of silently serving an unscoped search. Unreachable from the wire today: every in-tree backend refuses to create a vector index, so no integration test can construct one. It becomes reachable with the SQLite backend in #244, which is why this is worth fixing before that lands rather than after. Verification: 3 tests on `resolve_search_scope` plus 2 on the underlying validator. Proven discriminating by reintroducing the pre-fix guard, which fails `hash_index_searched_with_no_conditions_is_refused` in release builds, where the `debug_assert` is compiled out, so it is a logic failure rather than the assertion firing; both converse controls keep passing. fmt and `clippy --all-targets -- -D warnings` exit 0, 941 workspace tests, 0 filtered. Reported-by: robinnsc
LeeroyHannigan
requested review from
amrith, c33howard, jcshepherd, pdf-amzn, robinnsc and yesyayen
as code ownersAugust 19, 2026 10:49
robinnsc
approved these changes
Aug 19, 2026
Uh oh!
There was an error while loading. Please reload this page.
LeeroyHannigan added a commit
that referenced
this pull request
Aug 19, 2026
Implements vector indexes and SearchVectors on the SQLite backend, making it the first backend to declare the optional VectorSearchEngine capability added by the contract in #243. Working end to end: CreateTable with VectorIndexes; UpdateTable create and delete with a real backfill of items already in the table; DescribeTable reporting index status; exact-scan SearchVectors for COSINE, EUCLIDEAN and DOT_PRODUCT with per-metric ordering, partition scoping, TopK, inline equality filters and ProjectionExpression; index maintenance at all six write sites including the three TransactWriteItems branches; crash recovery that drops and rebuilds an index left CREATING; and table drop taking its vector data tables with it. Storage layout is one row per vector, chosen on write amplification rather than read speed. A packed per-partition blob reads 2 to 4 times faster but would rewrite the whole blob to insert one vector, roughly 390 MB per PutItem for a 100k-vector partition at 1024 dimensions. Recorded in docs/adr/0004-vector-search-exact-scan.md. The backfill status sequence follows what the service was measured to report rather than what seemed reasonable: CREATING with Backfilling false, then CREATING with true, then ACTIVE with the member absent. So presence does not imply backfilling and a client must read the value rather than test for the member. Also carries the measured missing-HASH refusal messages and their check precedence: the service distinguishes two distinct messages and validates HASH completeness before out-of-schema attributes. Probed live against us-east-1 rather than inferred. Breaking change, on-disk format: the SQLite catalog version moves to the version carrying the vector_indexes table, so an existing deployment must run `extenddb migrate` before the server will start. Linearised onto main as a single commit. The merge queue is configured to REBASE, which discards merge commits and replays the branch's original commits one at a time onto current main. Commit 9d97d04 predates #286 and re-fought a conflict in crates/engine/src/search_vectors.rs that the branch's merge of main had already resolved, so the queue refused the PR while the PR page reported no conflict. One commit has nothing to replay. Tree is byte-identical to the reviewed branch merged with main. Original commits, preserved here for reference: 8c53454 fix(vector): match the service's missing-HASH messages and check precedence 14b4fdf style: rustfmt after conflict resolution dc1d8d3 Merge remote-tracking branch 'origin/main' into vs244-fix 9d97d04 feat(sqlite): vector search index Verified before push: cargo fmt --all --check exit 0; cargo clippy --all-targets -D warnings exit 0 on both the default and sqlite feature sets; cargo test --workspace 1015 passed, 0 failed, 0 filtered out.
yesyayen pushed a commit
to yesyayen/extenddb
that referenced
this pull request
Aug 19, 2026
Implements vector indexes and SearchVectors on the SQLite backend, making it the first backend to declare the optional VectorSearchEngine capability added by the contract in ExtendDB#243. Working end to end: CreateTable with VectorIndexes; UpdateTable create and delete with a real backfill of items already in the table; DescribeTable reporting index status; exact-scan SearchVectors for COSINE, EUCLIDEAN and DOT_PRODUCT with per-metric ordering, partition scoping, TopK, inline equality filters and ProjectionExpression; index maintenance at all six write sites including the three TransactWriteItems branches; crash recovery that drops and rebuilds an index left CREATING; and table drop taking its vector data tables with it. Storage layout is one row per vector, chosen on write amplification rather than read speed. A packed per-partition blob reads 2 to 4 times faster but would rewrite the whole blob to insert one vector, roughly 390 MB per PutItem for a 100k-vector partition at 1024 dimensions. Recorded in docs/adr/0004-vector-search-exact-scan.md. The backfill status sequence follows what the service was measured to report rather than what seemed reasonable: CREATING with Backfilling false, then CREATING with true, then ACTIVE with the member absent. So presence does not imply backfilling and a client must read the value rather than test for the member. Also carries the measured missing-HASH refusal messages and their check precedence: the service distinguishes two distinct messages and validates HASH completeness before out-of-schema attributes. Probed live against us-east-1 rather than inferred. Breaking change, on-disk format: the SQLite catalog version moves to the version carrying the vector_indexes table, so an existing deployment must run `extenddb migrate` before the server will start. Linearised onto main as a single commit. The merge queue is configured to REBASE, which discards merge commits and replays the branch's original commits one at a time onto current main. Commit 9d97d04 predates ExtendDB#286 and re-fought a conflict in crates/engine/src/search_vectors.rs that the branch's merge of main had already resolved, so the queue refused the PR while the PR page reported no conflict. One commit has nothing to replay. Tree is byte-identical to the reviewed branch merged with main. Original commits, preserved here for reference: 8c53454 fix(vector): match the service's missing-HASH messages and check precedence 14b4fdf style: rustfmt after conflict resolution dc1d8d3 Merge remote-tracking branch 'origin/main' into vs244-fix 9d97d04 feat(sqlite): vector search index Verified before push: cargo fmt --all --check exit 0; cargo clippy --all-targets -D warnings exit 0 on both the default and sqlite feature sets; cargo test --workspace 1015 passed, 0 failed, 0 filtered out.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
robinnsc's non-blocking review finding on #243, fixed before #244 lands on top of it.
The defect
search_vectorsvalidated conditions against the index search schema only when the caller supplied a non-emptySearchConditionExpression:An index declaring a HASH element, searched with no expression, skipped validation and reached the backend with
hash_key: None.That contradicted two promises the contract makes:
VectorSearch::hash_keyincrates/storage/src/lib.rs, which tells backend authorsSomeis always populated when the index declares one, so they may treat it as a mandatory predicateA backend written against that doc would serve an unscoped search where a scoped one was promised: neighbours from every partition instead of the one the caller asked for.
The fix
The guard is gone; validation runs unconditionally. With no conditions the membership and type loops iterate nothing and the HASH-presence loop does the work, and an index with no search schema still accepts an absent expression.
Rather than leave the invariant resting on a comment, the validate-and-resolve step moved into
resolve_search_scope, which validates, splits conditions into partition scope and inline filters, and asserts a declared HASH element always yields a populatedhash_key. A future change reintroducing a conditional guard fails that assertion rather than silently serving an unscoped search.Reachability
Unreachable from the wire today: every in-tree backend refuses to create a vector index, so no integration test can construct one. It becomes reachable with the SQLite backend in #244, which is why this lands before that rather than after.
Verification
3 tests on
resolve_search_scopeplus 2 on the underlying validator. Proven discriminating by reintroducing the pre-fix guard:hash_index_searched_with_no_conditions_is_refusedfails in release builds, where thedebug_assertis compiled out, so it is a logic failure rather than the assertion firing. Both converse controls keep passing.fmtandclippy --all-targets -- -D warningsexit 0, 941 workspace tests, 0 filtered out.