feat(engine): HNSW probe widened to cosine + dot via per-index metric (SQLR-28)#113
Merged
Conversation
… (SQLR-28) Phase 7d.2's `try_hnsw_probe` was L2-only, so any KNN query using `vec_distance_cosine` or `vec_distance_dot` silently fell through to brute-force even with an HNSW index attached. Surfaced by the SQLR-23 v2 W10 bench: the HNSW variant clocked ~181 ms vs ~129 ms for brute-force because the cosine hot loop never touched the graph. Lands as sub-phase 7d.4 — per-index distance metric, no file format bump (the metric round-trips via the synthesized CREATE INDEX SQL in `sqlrite_master`): - New SQL surface: `CREATE INDEX … USING hnsw (col) WITH (metric = '<l2|cosine|dot>')`. Omitting the WITH clause defaults to L2, so pre-SQLR-28 catalogs round-trip byte-identical. Typo'd metric names error at CREATE INDEX time rather than silently defaulting to L2 — that silent fallback is exactly what we're fixing. - New `SqlriteDialect` (wraps sqlparser's `SQLiteDialect`, only override is `supports_create_index_with_clause = true`). - `HnswIndexEntry` grows a `metric: DistanceMetric` field; the load, rebuild, and dirty-rebuild paths all consume the per-entry metric instead of hard-coded L2. - `try_hnsw_probe` widens to all three `vec_distance_*` functions and only fires when the index entry's metric matches the query function. Mismatch → brute-force fallback (correct, just slow). - W10 bench bumped to v3; the HNSW variant creates the index `WITH (metric = 'cosine')`. v1/v2 numbers are not comparable. - Tests: cosine + dot self-query through the optimizer, metric-mismatch fallback, unknown-metric rejection, WITH-on-btree rejection, save+reopen preserves cosine metric. Unblocks SQLR-25 (republish v2/v3 bench numbers). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
try_hnsw_probewas L2-only, so any KNN query usingvec_distance_cosineorvec_distance_dotsilently fell through to brute-force even with an HNSW index attached. Surfaced by the SQLR-23 v2 W10 bench (~181 ms HNSW vs ~129 ms brute-force on a cosine hot loop because the graph was never being touched).CREATE INDEX … USING hnsw (col) WITH (metric = '<l2|cosine|dot>'). Defaults to L2 when omitted, so pre-SQLR-28 catalogs round-trip byte-identical. The metric round-trips via the synthesized CREATE INDEX SQL insqlrite_master— no file format bump.try_hnsw_proberecognises all threevec_distance_*functions and only fires when the index's metric matches the query's function; mismatches fall through to brute-force (correct, just slow).WITH (metric = 'cosine')); v1/v2 numbers are not comparable to v3 and have been retired inbenchmarks/README.md.Unblocks SQLR-25 (republish v2/v3 bench numbers).
Approach (option (a) from the SQLR-28 ticket)
Per-index metric persisted via the CREATE INDEX SQL, not via a file format bump:
src/sql/dialect.rs—SqlriteDialectwraps sqlparser'sSQLiteDialect, overrides onlysupports_create_index_with_clause = trueso the parser accepts the WITH clause.HnswIndexEntrygrows ametric: DistanceMetricfield.create_hnsw_index,rebuild_hnsw_index, andrebuild_dirty_hnsw_indexesall consume the per-entry metric instead of hard-coded L2.synthesize_hnsw_create_index_sqlappendsWITH (metric = '…')for non-L2 entries; L2 omits the clause to keep the pre-SQLR-28 round-trip identical.Test plan
cargo build --workspace(excl. desktop / python / nodejs / benchmarks)cargo test --workspace— 481 engine + 12 main + 20 ask + 19 mcp + … all greencargo fmt --all -- --checkcargo clippy --workspace --all-targets— clean (only pre-existing FFI doc warnings)cosine_self_query_through_hnsw_optimizer—WITH (metric = 'cosine')index + cosine query returns the self-vector as nearestdot_self_query_through_hnsw_optimizer— same shape for dotmetric_mismatch_falls_back_to_brute_force— L2 index + cosine query still returns correct nearestunknown_metric_name_is_rejected—'cosin'errors at CREATE INDEX timewith_metric_on_btree_is_rejected— WITH on a non-HNSW index errorsround_trip_preserves_hnsw_cosine_metric— save + reopen restores the cosine metric on the loaded entryFollow-ups
m/ef_construction/ef_searchknobs from Phase 7 plan Q2 remain deferred. SQLR-28 only landed the metric knob.🤖 Generated with Claude Code