Skip to content

feat(sqlite): vector search index - #244

Merged
LeeroyHannigan merged 1 commit into
mainfrom
feat/sqlite-vector-search
Aug 19, 2026
Merged

feat(sqlite): vector search index#244
LeeroyHannigan merged 1 commit into
mainfrom
feat/sqlite-vector-search

Conversation

@LeeroyHannigan

Copy link
Copy Markdown
Collaborator

What

Implements vector indexes and SearchVectors on the SQLite backend, making it the first backend to declare the optional VectorSearchEngine capability added by the contract #243 this is stacked on.

Working end to end:

  • CreateTable with VectorIndexes, and UpdateTable create/delete with a real backfill of items already in the table
  • DescribeTable reports the indexes and their 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: a vector index left CREATING is dropped and rebuilt at startup
  • Cleanup: dropping a table takes 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 the ADR.

The backfill status sequence follows what the service was measured to report, not what seemed reasonable: CREATING with Backfilling: false, then CREATING with true, then ACTIVE with the member absent. Observed on 2026-08-06 by seeding 3000 items of 1024 dimensions so the backfill took 8.5 minutes and could be sampled. false comes first, so presence does not imply backfilling and a client must read the value rather than test for the member.

Why

The contract PR models the vector index surface but no backend implements it, so no vector index can be created in tree. This makes it real on SQLite, which is the backend that ships in the dev/test image.

It also closes a hole the contract's post-condition guard exposed: this backend declared the capability while ignoring vector_index_updates, so an UpdateTable create returned 200 and created nothing, and a delete returned 200 while the index stayed ACTIVE and kept returning hits.

Stacked on the vector contract PR. Review that one first.

Closes #

Testing done

Verified against a live SQLite-backed server over HTTP, not in process:

# server on the SQLite backend, throttling enforcement on as CI does
extenddb init --backend sqlite && extenddb settings set throttling_enabled true
extenddb serve
cd tests/rust && EXTENDDB_EXPECT_VECTORS=1 cargo test -- --test-threads=1
# 445 passed; 0 failed; 0 filtered out
cargo test # 773 passed; 0 failed; 0 filtered out
cargo fmt --all --check # clean
cargo clippy --all-targets -- -D warnings # clean
cargo clippy --all-targets --no-default-features --features sqlite -- -D warnings # clean

25 vector wire tests, covering nearest-first ordering, TopK, overwrite, delete, unindexed items, tenant isolation in both directions, partition moves when the HASH attribute changes, the dot-product ordering inversion, backfill of pre-existing
items (including one without a vector that must be skipped rather than break the scan), later writes still indexed, index delete stopping both serving and reporting, base items surviving an index delete, duplicate-create and missing-delete rejection, and the f32 narrowing.

Three things were mutation-checked rather than trusted green:

  • Neutering the backfill to write nothing fails both backfill tests, and the second then reports only the later-written item, so it genuinely distinguishes backfilled rows from live-written ones.
  • Removing the drop from the reconciler fails the partial-table test. That test exists because the crash that actually happens leaves the data table holding some rows, and resuming would index those items twice; the simpler no-table crash case cannot catch it, since the drop is a no-op there.
  • Weakening the unscoped-partition sentinel fails the invariant test

Also verified directly against the database rather than inferred:

  • After the full suite: 21 vector data tables against exactly 21 catalog rows, zero orphans, zero missing.
  • The catalog version bump was exercised by migrating a live deployment 0.0.2 to 0.0.3 and confirming the server starts.
  • Row payload at 1024 dimensions is 37 bytes against a 4,096-byte blob, down from 19,862 bytes before the vector was removed from the payload.

Measured scan throughput is roughly 1,000 vectors per 10 ms at 1024 dimensions on one core. Worth stating plainly because an earlier modelled figure of 36,000 was wrong by 10 to 30 times: the cost is the SQLite read path, not the arithmetic, and a zero-copy &[f32] view measured no faster.

Checklist

  • I have read CONTRIBUTING.md
  • All tests pass (cargo test --workspace)
  • Code is formatted (cargo fmt --check)
  • Clippy is clean (cargo clippy -- -W clippy::pedantic)
  • I have added or updated tests for new functionality
  • I have updated documentation if behavior changed
  • Breaking changes are noted below (if any)
  • If this changes the wire protocol, Storage trait, auth model, on-disk
    format, or public CLI surface, an RFC has been accepted or is linked
    below. Otherwise, an ADR captures the decision (link below).

ADR / RFC: docs/adr/0004-vector-search-exact-scan.md (added here), RFC #236

Breaking changes

On-disk format. The SQLite catalog version moves from 0.0.3 to the version carrying the vector_indexes table, so an existing deployment must run extenddb migrate before the server will start. The migration is the idempotent schema apply; it was exercised on a live deployment rather than reasoned about.

No wire-protocol or trait changes here: those are all in the contract PR.

Known gaps, stated rather than discovered

  • Index maintenance is synchronous, where the service is eventually consistent like a GSI. Being fresher cannot return a wrong answer, so this is safe, but it is a parity divergence and it keeps vector work on the write's critical path. The async path should reuse the existing gsi_pending queue, which already has crash recovery and per-key FIFO. Not in this PR yet.
  • Wire coverage is still missing for BatchWriteItem, composite base keys, the scoped-index SearchConditionExpression requirement, and wrong-dimension requests.

By submitting this pull request, I confirm that my contribution is made under
the terms of the Apache License 2.0 and I agree to the Developer Certificate of
Origin (DCO). See CONTRIBUTING.md for details.

@LeeroyHanniganLeeroyHannigan changed the title Feat/sqlite vector searchfeat(sqlite) vector searchAug 7, 2026
@LeeroyHanniganLeeroyHannigan changed the title feat(sqlite) vector searchfeat(sqlite): vector search indexAug 7, 2026
@LeeroyHannigan
LeeroyHanniganforce-pushed the feat/sqlite-vector-search branch from c37f379 to 6031dafCompareAugust 10, 2026 20:44
@LeeroyHannigan
LeeroyHannigan changed the base branch from main to feat/vector-search-contractAugust 12, 2026 12:40
@LeeroyHannigan
LeeroyHannigan marked this pull request as ready for review August 12, 2026 14:56
yesyayen
yesyayen previously requested changes Aug 13, 2026

@yesyayenyesyayen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: where must the index-build lifecycle code live?

The GSI build is synchronous and backend builds the index in one transaction, and the table is locked for writes until the build is complete. Each backend has its own copy of this logic, and each copy is approximately 100 lines.

The vector backfill runs as a detached task, and the table stays open for writes during the build. Because writes can occur during the build, this PR adds approximately 1,500 lines of new logic: the CREATING to ACTIVE state machine, the queue hold and replay, the poison-row handling, the watchdog, and the startup reconciliation.
When postgres implements VectorSearchEngine, a second copy of all this logic is necessary. The GSI code shows what occurs when a backend keeps its own copy. The two backfill_gsi copies do not agree today: the postgres copy does not report CREATING, it has no crash reconciler, and it keeps the all-0xFF bound defect that this PR removed from sqlite.

My proposal is this: put the state machine in shared engine code, and let each backend supply only the storage primitives (scan a batch, insert a row, drop a table). If the decision is to do this later, lets write that decision in ADR-0004.

Comment threadcrates/storage-sqlite/src/data/vector_index.rs
Comment threadcrates/storage-sqlite/src/workers.rs
LeeroyHannigan added a commit that referenced this pull request Aug 13, 2026
…xpression
Review finding from yesyayen on #243: `vector_relevant_assignments` decided
what to validate by matching expression FORM, and only matched a bare
`SET emb = :v`. The doc comment admitted the rest ("complex right-hand
sides (arithmetic, `if_not_exists`, nested paths) are left for the storage
layer"), but the storage layer was not validating them either, so
`SET emb = list_append(:a, :b)`, `SET emb = other_attr` and
`SET emb = if_not_exists(emb, :v)` all skipped validation entirely and
could store a malformed or wrong-dimension list under a vector attribute.
The consequence is worse than a missing 400. At propagation delay 0 the
user gets a 500 instead of a ValidationException; at delay > 0 the write
returns 200 and the propagation worker silently drops the pending row,
leaving the index permanently stale with nothing surfaced to anyone.
The reviewer's proposed fix is the right one and is adopted here: validate
the computed post-update image, so no current or future SET syntax can
bypass the check. Vector validity is a property of the stored value, not
of the expression that produced it.
Two findings while implementing it, beyond what was reported:
- The same hole exists on the TransactWriteItems update path via
`assigned_vector_attributes`, which was not mentioned in the review.
- `apply_update` has EIGHT call sites across the three backends
(update_item and transactions in sqlite and postgres, four in
mongodb), so adding the check at each would have been eight copies of
a rule that must not be forgotten -- the duplication problem raised
separately in #244's review.
So the check is enforced in one place instead. `expression::apply_update`
gains a sibling, `apply_update_validated`, which applies the actions and
then validates the resulting image, and all eight backend call sites now
use it. Forgetting the check is no longer possible for a future backend
that applies an UpdateExpression, which is the property worth having.
The engine-level form check in UpdateItem is removed rather than left in
place, because keeping it would mean two competing models for the same
property and it implied coverage it did not have. The comment there now
says why validation deliberately does not live in the engine.
The TransactWriteItems pre-flight is KEPT, and its doc comment now says
what it is for: it produces the correct per-item CancellationReason vector
for the bare-placeholder case, which the storage layer cannot reproduce
because it aborts at the first failing operation. It is explicitly not the
authoritative check, and the guarantee is documented as living in
apply_update_validated.
Placement follows existing convention rather than inventing one: sqlite
and mongodb already validate secondary-index key types on the post-update
image in these same functions, with comments noting that the post-update
item is what actually gets written. Vector validation was the outlier.
Tests: 9 new cases in the evaluator, covering each form that previously
bypassed validation (bare placeholder, list_append, if_not_exists,
attribute copy, append overflowing an already-full vector) plus the cases
that must still be ALLOWED (list_append reaching the exact dimension,
REMOVE of the vector attribute, an unrelated update, absent vector), and
search-schema validation on the image.
Negative control run: with the helper reverted to plain `apply_update`,
exactly the 5 rejection tests fail and the 4 allow-tests still pass, so
they discriminate on the fix rather than passing incidentally. One earlier
draft test was discarded for failing this bar: `ADD emb :bad` errored
inside apply_update regardless of validation, so it proved nothing, and it
was replaced with the append-overflow case which apply_update accepts.
Gates: fmt --check exit 0, clippy --all-targets -W clippy::pedantic exit 0
with 0 errors, 809 lib tests passed / 0 failed / 0 filtered out.
LeeroyHannigan added a commit that referenced this pull request Aug 14, 2026
…or scaffolding
Rewrite of the earlier R2 commit after checking the child branch. The first
version re-implemented four validation rules on the create path that
feat/sqlite-vector-search (#244) already enforces, one of them WORSE: #244
treats an absent BillingMode as PROVISIONED (the API default) and rejects,
where this branch's version only rejected an explicit PROVISIONED, letting
an omitted billing mode through. Create-path enforcement therefore stays
with #244, whose version wins on merge; this commit carries only what #244
lacks.
What this commit actually adds:
- UPDATE-path attribute-definition enforcement, the one behavioural gap on
both branches: `validate_vector_index_updates` now takes the request's
AttributeDefinitions and applies the two shared rules to every created
index (the vector attribute must NOT be declared; every SearchSchema
element MUST be). Measured 2026-08-13: on UpdateTable the definition must
be in THAT request even when the attribute is already declared on the
table, which is what the test pins. Wired in engine/update_table.rs.
- The shared checker `validate_vector_index_attribute_definitions`, written
once so create (on #244, after merge) and update apply identical rules.
- Named message constants for the measured service strings
(VECTOR_INDEX_REQUIRES_PAY_PER_REQUEST, VECTOR_INDEX_COUNT_LIMIT_CREATE,
VECTOR_SEARCH_SCHEMA_UNDECLARED, vector_attribute_conflicting_definition):
the named form of what #244 currently inlines, so the merge can dedupe to
one home. Byte-checked against the captured service responses
programmatically, since Rust string continuations elide whitespace and a
mismatch is invisible to eyeball review.
- Scaffolding for the three STATE-DEPENDENT UpdateTable rules that need the
table's catalog and therefore land in the backends on #244, not here:
a LimitExceededException error variant (measured: adding a sixth index
via UpdateTable returns LimitExceededException "Subscriber limit
exceeded: Number of vector secondary indexes exceeds per-table limit of
5", a DIFFERENT CLASS AND TEXT from CreateTable's ValidationException),
the VECTOR_INDEX_COUNT_LIMIT_UPDATE constant carrying that text, and
vector_attribute_redefines_key for the key-collision message that embeds
both schemas. These are intentionally not constructed in this commit;
the backend enforcement commits on #244 construct all three. If those
commits do not land, these should be reverted rather than shipped inert.
Also fixes the clippy failure CI caught on the previous version of this
commit (empty-line-after-doc-comment at validation/mod.rs:1676): CI runs
`cargo clippy --all-targets -- -D warnings`, which turns that warning into a
hard error, while the local gate had used pedantic without -D and sailed
past it. Local gates now use CI's exact flags.
Evidence for every measured message: /home/lhnng/.meshclaw/workspace/
vector-validation-rules.md (probes against us-east-1, 2026-08-13, including
the confounded probe that was re-run).
Gates (CI flags): fmt --check 0, clippy --all-targets -D warnings 0,
lib tests 827 passed / 0 failed / 0 filtered out.
LeeroyHannigan added a commit that referenced this pull request Aug 14, 2026
…nd backend
Answers the architectural question from the #244 review: where should the
vector index-build lifecycle live, given the GSI precedent of per-backend
copies that have already diverged?
The reviewer proposed a shared engine state machine with backends supplying
storage primitives, or an ADR recording the deferral; the reviewer's "ADR-0004"
slot is taken (vector-search-exact-scan), so this is 0005.
Decision: defer the extraction until a second backend implements
VectorSearchEngine, and make it the FIRST task of that work, with a hard
review gate against a second lifecycle copy. All three of the reviewer's
claimed GSI divergences were verified in the tree and are cited with
file:line as the evidence the forcing function exists for. The core
argument for deferring: a shared abstraction extracted from one
implementation encodes that implementation's accidents (rowid cursors,
process-wide write lock, BEGIN IMMEDIATE semantics) as the interface, and
which of those are essential only becomes visible against a real second
backend. The behaviour a future extraction must preserve is already pinned
by discriminating tests (backfill, hold-and-replay, poison-skip,
transient-retry, reconciliation).
@LeeroyHannigan

Copy link
Copy Markdown
CollaboratorAuthor

Thanks for this review — every finding held up, and two turned out worse than reported. All six are addressed across both branches.

R3 (if_not_exists bypass) was the most severe: the expression-form check also missed the identical hole on TransactWriteItems, and apply_update turned out to have eight call sites across the three backends. The fix is one shared apply_update_validated in core that all eight use, validating the post-update image (the same convention the backends already use for secondary-index keys), so a future backend cannot forget it. Nine tests, negative-controlled: with the wrapper reverted, exactly the rejection tests fail.

R1 (capacity) was half right — search-side existed, write-side had zero occurrences. Rather than guess, I measured against the real service (20 probes): VectorWriteRequestBytes = max(dimensions * 4 + projected_non_vector_bytes, 1024), doubled when the search-schema HASH value moves the entry between partitions. Two things the public docs get wrong, both measured: the charge follows the PROJECTED entry changing (a non-indexed attribute change is charged under ALL, not under KEYS_ONLY), and VectorIndexes is reported for INDEXES only, not TOTAL. Wired through Put/Update/Delete with six wire tests pinning exact figures. One honest gap remains: BatchWriteItem and TransactWriteItems aggregate per-table units without item images, so they don't yet report vector charges — the same documented follow-up that already covers their missing per-GSI breakdowns, not a claim of support.

R2 (validation gaps) was real but the rules already existed on the child branch with a better billing-mode check (absent BillingMode defaults to PROVISIONED and is rejected). The contract branch now carries only what was genuinely missing — the update-path enforcement, where the service diverges from create: the sixth index is LimitExceededException on UpdateTable vs ValidationException on CreateTable, and the key-collision message embeds both schemas. All messages probed byte-exact.

R5 (poison rows) was worse than reported: one malformed item also froze all async index maintenance for the table via the CREATING hold. Backfill now skips, counts, and surfaces poison rows instead of wedging.

R6 — transient errors now propagate so the worker's existing retry-next-pass loop handles them; only genuinely malformed items are dropped, with classification at the drop site.

R4 — agreed on the divergence risk, but with one backend implementing the lifecycle today the shared state machine would be an abstraction with a single consumer (~490 lines moved speculatively). Recorded as ADR-0005 (0004 was taken) with the concrete trigger: the machinery moves to the engine when a second backend implements vector indexes, and the Postgres divergences you cite are listed there as the evidence.

Happy to walk through the capacity probes if useful — the raw figures are in the commit.

robinnsc
robinnsc previously approved these changes Aug 19, 2026

@robinnscrobinnsc left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving with one real finding flagged inline (a collision between inline maintenance and an in flight backfill when the propagation delay is 0), plus two small notes:

  1. On the UpdateTable delete path the catalog row is removed inside the transaction but the data table drop happens after commit. If the drop errors, the caller gets an error for a delete that already took effect, and the data table is orphaned with nothing that ever reclaims it, since the reconciler only looks at CREATING rows. Probably the same shape as the GSI sibling, but worth a tracking note.
  2. The two consecutive sightings rule in the stuck build sweep depends on the registry insert in build_vector_index staying before the spawn. The sweep side explains this nicely; the registration site does not mention that the sweep relies on its ordering, so a future refactor could move it without knowing what breaks. One sentence there would tie them together.

Comment on lines +461 to +466
// A plain INSERT, deliberately, where the GSI sibling uses INSERT OR REPLACE.
// Every caller reaches this through `apply_vector_index`, which unconditionally
// deletes the base key's row first, so no live row can exist here and a conflict
// is impossible. Keeping it a plain INSERT means that if a future refactor ever
// makes that delete conditional, this fails loudly with a primary key violation
// rather than silently replacing a row and hiding the broken invariant.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment is not quite true, and the gap behind it is reachable. The backfill calls insert_vector_row directly from backfill_vector_batch, without going through apply_vector_index, so there is a caller that never does the preceding delete.

That matters once writes can land during a backfill. The CREATING hold protects the queued path, but with index_propagation_delay_ms set to 0 (the documented synchronous mode) maintain_vector_indexes applies inline, and fetch_vector_indexes_for_table has no status filter, so a live write inserts a row into the still CREATING index's table. When the backfill later reaches that base rowid, this plain INSERT hits a primary key violation, the batch fails, and the index sits in CREATING until the stuck build sweep rebuilds it. Under sustained writes at delay 0 the rebuild can keep colliding, so the index may never reach ACTIVE while searches are refused the whole time.

Two ways out, either seems fine. The inline path could treat CREATING indexes the way the queue does and enqueue instead of applying, so the hold covers both paths uniformly. Or the backfill could tolerate an existing row, since a row present at backfill time was written from a base value at least as new as the one the batch just read, so replacing or skipping are both sound. That trades away the loud failure property this comment argues for, so the first option feels more in keeping with the design.

Not blocking since the default delay is nonzero and the window needs a write during a backfill, but the failure mode is an index that never activates, which is worth closing before this pattern gets copied to the Postgres backend.

@LeeroyHannigan
LeeroyHannigan added this pull request to the merge queueAug 19, 2026
@github-merge-queue
github-merge-queueBot removed this pull request from the merge queue due to a conflict with the base branch Aug 19, 2026
@LeeroyHannigan

Copy link
Copy Markdown
CollaboratorAuthor

@robinnsc your approval got auto-dismissed by one more commit (8c53454), so this needs a re-approve. The delta since what you reviewed is small and self-contained: while resolving the merge I probed the service and found it distinguishes the two missing-HASH failure modes, so the guard now fires only when no expression is supplied, the validator carries the full measured message for an expression that omits the HASH ('...in configured SearchSchema'), and the HASH-completeness check runs before the out-of-schema check to match the service's precedence (probed both orderings today). One new unit test pins the two-message split. Full gates on the tip: fmt/clippy clean both feature sets, 970 lib tests 0 filtered, 45/45 vector integration live.

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.
@LeeroyHannigan
LeeroyHanniganforce-pushed the feat/sqlite-vector-search branch from 8c53454 to 4c918b5CompareAugust 19, 2026 16:09
@yesyayen
yesyayen self-requested a review August 19, 2026 16:26
@LeeroyHannigan
LeeroyHannigan added this pull request to the merge queueAug 19, 2026
Merged via the queue into main with commit 5eda17eAug 19, 2026
18 checks passed
yesyayen pushed a commit to yesyayen/extenddb that referenced this pull request Aug 19, 2026
…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 ExtendDB#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
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@LeeroyHannigan@yesyayen@robinnsc