Repository navigation
fix(vector): ORDER BY vec_distance_*() LIMIT k reaches the vector index, and CREATE VECTOR INDEX builds one - #474
Merged
Conversation
…ng in a dead branch `CREATE VECTOR INDEX` and the `ORDER BY vec_distance_*(col, vector) LIMIT k` planner branch were both permanently inactive, and restoring the deleted method alone would have revived neither. `SqlParser.VectorQueryOptimizer` was an `internal static` that no code in SharpCoreDB ever assigned and no other assembly could assign (the parser is internal), while SharpCoreDB.VectorSearch registers the optimizer with the DI container. So `VectorQueryOptimizer?.BuildIndex(...)` in the DDL path and `VectorQueryOptimizer is not null` in the SELECT path were no-ops in both directions: nothing built an index and nothing looked for one. Wiring: Database.GetSharedSqlParser now resolves IVectorQueryOptimizer from the service provider — the same pattern GRAPH_RAG already used — and attaches it via SetVectorQueryOptimizer; the property is per parser instance instead of static. The second SqlParser construction site (the compiled-query fallback) now goes through GetSharedSqlParser, so falling back cannot produce a parser without the extensions the first site attaches. Restored and repaired: TryExecuteVectorOptimized was replaced by a stub returning null in 997103b, after working in e220461. The restored code is not a snapshot of it. The old version copied every column and appended a "distance" key whatever the SELECT list said, and did so for DESC and WHERE queries too, so an optimized query and an unoptimized one returned different shapes for the same SQL. Rows are now projected onto the SELECT list (SELECT *, plain columns, aliases, and the distance expression substituted from the key the optimizer injects), ORDER BY may name either the alias or the function, and the two shapes an ascending top-k index cannot answer — DESC and a WHERE clause — fall back to the scan instead of returning a plausible wrong answer. Found while testing: an unquoted vector literal (`[1,0,0,0]`, which is exactly what ParameterBinder produces for a float[] parameter) lost its commas to the argument splitter, leaving "[1" as the query vector. Brackets and braces now count as nesting, so a bound float[] parameter works end to end. Tests: tests/SharpCoreDB.VectorSearch.Tests/SqlVectorQueryTests.cs, seven end-to-end cases through AddVectorSupport() + Database — the only level at which the wiring is observable, which is why the existing VectorQueryOptimizer unit tests stayed green throughout the outage. 255/255 in that project, 2137/2137 in SharpCoreDB.Tests. Known limitation, unchanged: an index is a snapshot of the table at CREATE VECTOR INDEX; rows inserted afterwards are not in it until it is rebuilt.
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
|
MPCoreDeveloper
added a commit
that referenced
this pull request
Oct 25, 2026
Release 2.1.1: version bump, release notes and changelog (stacked on #474)
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.
Symptom
ORDER BY vec_distance_cosine(embedding, @q) LIMIT kandCREATE VECTOR INDEXboth did nothingobservable: the plan fell back to a full scan, and the index statement returned without building an
index. Nothing threw, so both read as "the optimizer declined" rather than "the optimizer was never
reached".
Two independent breaks, not one
SqlParser.TryExecuteVectorOptimizedreturnednullunconditionally.
e220461ehad a working ~100-line implementation (argument extractor included);997103bereplaced it with the stub and four unused parameters.VectorQueryOptimizeritself wasintact — only the call path was gone.
SqlParser.VectorQueryOptimizerwas aninternal staticproperty that no code in the engine assigned, and no other assembly could assign it (the parser is
internal).SharpCoreDB.VectorSearchonly registers the optimizer with the DI container. Bothconsumers were null-conditional, so
CREATE VECTOR INDEXwas dead for the same reason the plannerwas — the whole SQL surface, not just the ranking branch.
Fixing only one of them leaves the symptom in place, which is why the existing
VectorQueryOptimizerunit tests stayed green throughout: they exercise the optimizer directly, never through
Database.What changed
Database.GetSharedSqlParserresolvesIVectorQueryOptimizerfrom the service provider(the same pattern
IGraphRagProvideralready used) and attaches it per parser instance viaSetVectorQueryOptimizer. The duplicatenew SqlParser(...)in theExecuteCompiledQueryfallbacknow routes through
GetSharedSqlParser, so there is one construction point instead of two.TryExecuteVectorOptimizedprojects onto the SELECT list (SELECT *,aliases and the distance expression substituted), accepts an
ORDER BYthat names either the alias orthe function, and refuses
DESCand a non-emptyWHERE— an ascending top-k cannot answer those, andreturning the wrong shape silently is what made the original bug hard to see. Those two fall back to a
scan.
float[]argument is sent unquoted as[1,0,0,0], and the argument splitternow tracks bracket and brace nesting, not just parentheses — a query vector is not a call argument.
Tests
New
SqlVectorQueryTestsgo throughAddVectorSupport()+Database+ SQL, the only level where thewiring is observable, covering: index built by
CREATE VECTOR INDEX,ORDER BY vec_distance_cosinewith a literal and a parameter vector, alias form,
SELECT *form,DESCandWHEREfalling back to ascan, and a query on an index that does not exist.
SharpCoreDB.VectorSearch.Tests: 255/255SharpCoreDB.Tests: 2137/2137 (16 skipped)Known limitation
An index is a snapshot taken at
CREATE VECTOR INDEX. Inserts after it are not in the index until it isrebuilt; the scanner path still sees them, so a stale index under-answers rather than mis-answers.