Skip to content

fix(datafusion): return vector_search rows in best-first relevance order#613

Open
XiaoHongbo-Hope wants to merge 6 commits into
apache:mainfrom
XiaoHongbo-Hope:fix-vector-search-tvf-ordering
Open

fix(datafusion): return vector_search rows in best-first relevance order#613
XiaoHongbo-Hope wants to merge 6 commits into
apache:mainfrom
XiaoHongbo-Hope:fix-vector-search-tvf-ordering

Conversation

@XiaoHongbo-Hope

@XiaoHongbo-Hope XiaoHongbo-Hope commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Makes the DataFusion vector_search table function return rows in the index's
best-first relevance order (previously it re-read rows in physical file order, so the
first row was often not the most similar).

What it does

  • Materializes the scored row-ids (best-first) from execute_scored, reads the matching
    rows (projection pushdown + internal _ROW_ID), then gathers them back into relevance
    rank and drops _ROW_ID. Output schema is unchanged.
  • Casts columns to the provider schema (e.g. Utf8 → DataFusion Utf8View) so string
    columns round-trip.
  • Runs the search + read + gather in a custom execution-time VectorSearchExec (not at
    planning time), so EXPLAIN stays cheap and the work runs under the TaskContext.
  • Keeps the full top-k for the ANN search and applies the outer LIMIT by truncating the
    already-ranked result before reading, so recall is unchanged and a large top-k with a
    small outer LIMIT only reads what it returns.

Dependency / known limitation — needs #614
This PR makes the scan honor the order of SearchResult.row_ids, but that order is only
fully correct once #614 is merged. On current main, SearchResult::top_k does not
sort when candidate_count <= k (i.e. limit >= number of matched rows), so for that
case the index returns rows in scored-map order and this scan faithfully reproduces it.
With #614, the ranking is correct for all limits.

Because of that, the string-column test asserts its hit set order-independently (the cast
is what it verifies); deterministic relevance ordering is covered by
test_vector_search_orders_by_relevance, which uses a top-k smaller than the candidate
count (the sorted/truncation path). Merge order: #614 first, then rebase this PR for
the full-ordering guarantee across all limits.

The `vector_search` table-valued function materialized its results with a
plain row-range scan, so `SELECT * FROM vector_search(...)` returned the
matching rows in physical file order instead of by relevance. This
contradicts the documented contract ("returns the top-k rows ordered by
relevance score") and means the first returned row is often not the most
similar one, especially at larger `limit`s.

Materialize the scored row-ids (best-first) via `execute_scored`, read the
matching rows (with projection pushdown, plus the internal `_ROW_ID`
column), then realign them to the index's relevance rank via a gather and
drop `_ROW_ID`. A scored row-id that is not materialized fails loud rather
than silently shrinking the top-k. The output schema is unchanged (no score
column exposed). `execute_read` is intentionally not reused: it is
statically `!Send` due to the primary-key-vector path, which the async
`TableProvider::scan` cannot hold.

Add a regression test that builds an ivf-flat vindex table and asserts the
returned order is `[2, 5, 1]` (relevance order), which differs from the
ascending-id file order `[1, 2, 5]` the old code produced, covering both the
projected (`SELECT id`) and full (`SELECT *`) paths.
@XiaoHongbo-Hope
XiaoHongbo-Hope force-pushed the fix-vector-search-tvf-ordering branch from 11afe92 to 78913fd Compare July 26, 2026 13:00
The gather assembled the reordered result with the provider's Arrow schema
(where string columns are DataFusion `Utf8View`) but kept the Paimon read's
column types (`Utf8`), so a `vector_search` query returning a string column
could fail on the type mismatch. Cast each column to the output field type
when it differs, matching the normal scan path's `to_datafusion_batch`.

Add a regression test with a `VARCHAR` column asserting the query executes
and stays relevance-ordered.
…dent

test_vector_search_casts_string_column asserted a specific relevance order for
a 3-row fixture, which is not stable across environments for such a tiny index
(the exact-match hit can rank either first or second). Its purpose is the
string-column schema cast, not ordering (covered by
test_vector_search_orders_by_relevance), so assert the hit set order-independently.
The scan previously ignored DataFusion's pushed-down limit, so an outer
`LIMIT n` over a large top-k (e.g. `vector_search(..., 1_000_000) LIMIT 1`)
still searched, read, and materialized every row before truncating —
a coordinator OOM risk. Bound the search by `min(top_k, outer_limit)` so the
read and in-memory materialization only cover the rows the query can return.

Add a test asserting an outer LIMIT returns the correctly-ranked top-n.
…correctly

Two fixes to the vector_search scan:

- The previous commit applied the outer LIMIT by shrinking the search's top-k
  (`with_limit(min(k, outer))`). That changes the ANN search width (list size,
  refine candidates), so `vector_search(..., 100) LIMIT 1` is not the first row
  of the top-100 search. Instead keep the full top-k for the search and truncate
  the already-ranked result before reading rows — the recall is unchanged and a
  large top-k with a small outer LIMIT only reads/materializes what it returns.

- Move the search + read + rank-order gather out of `scan()` (planning time) into
  a custom single-partition `VectorSearchExec` whose stream does the work at
  execution time. Planning and `EXPLAIN` no longer read or materialize anything,
  and the work runs under DataFusion's `TaskContext`.
Reject non-empty children in with_new_children (the exec is a leaf) and reject
a non-zero partition in execute (it has a single partition), instead of
silently ignoring them.
Sign up for free to 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.

1 participant