perf: plan take operations through FilteredReadExec's range-read path#7672
Open
LuQQiu wants to merge 6 commits into
Open
perf: plan take operations through FilteredReadExec's range-read path#7672LuQQiu wants to merge 6 commits into
LuQQiu wants to merge 6 commits into
Conversation
TakeExec reads rows through the v2 readers' point-lookup path with serial
per-fragment opens; reading the same rows through FilteredReadExec's planned
range reads is 2.3-2.7x faster (take-100 benchmark, warm NVMe).
Generalize FilteredReadExec's input to "a plan that tells the node which rows
to read", accepting two encodings detected from the input plan's schema:
- the serialized IndexExprResult mask batch (existing behavior, unchanged)
- any plan carrying a _rowid/_rowaddr column ("take mode", new): each batch
is converted to an in-memory IndexExprResult and read through the same
plan_scan -> read_fragment pipeline, then the columns are merged back into
the input batch preserving row order, duplicates, and payload columns
(e.g. _distance) - the same contract as TakeExec
Scanner::take and merge_insert now plan takes as FilteredReadExec on the v2
storage format. TakeExec is unchanged and still used for legacy (v1) storage.
The CoalesceTake optimizer rule also collapses stacked take-mode reads, and
count-pushdown / distributed proto serialization explicitly skip take-mode
nodes (distributed support is a follow-up).
Adds a three-arm benchmark (TakeExec vs mask mode vs take mode) at
rust/lance/benches/take_exec_compare.rs.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
gen_range is deprecated and fails clippy -D warnings. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Box the TakeRows variant (large size difference between variants) and allow print_stdout in the benchmark like the other benches. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A random take touches many fragments (often one row per fragment) and try_join_all drives the opens concurrently but on a single task, so the CPU-bound part of ~100 fragment opens ran back to back (~4ms/query on a many-fragment dataset). Spawn one task per fragment open and per decode, matching the scan path (FilteredReadStream::try_new). Local take-100 (2M rows, 20 fragments): take mode 671 -> 858 QPS, now within 7% of mask mode (920 QPS) vs TakeExec at 445 QPS. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Review feedback: the node read as if it had "modes" (mode=take display, is_take() checks). Reframe it as one general I/O node, output = read(row_source, fields_to_read) + carry_columns, where only the row source varies: AllRows (no input), RowSet (one serialized IndexExprResult batch; sets are unordered and unique by nature), or RowStream (record batches whose order, duplicates, and columns are preserved). The source kind is derived from the input plan's schema, never configured. Renames only, no behavior change: FilteredReadInput -> RowSource, TakeRowsInput -> RowStreamSource, is_take() -> row_stream_input(), mode=take(_rowid) -> source=stream(_rowid) in plan text, and consumer checks (count pushdown, proto, CoalesceTake) now ask attributes instead of node identity. Also documents the legacy-storage invariant at both TakeExec swap sites (the v1 reader cannot serve FilteredRead) and the deferred count-through-streams rewrite in count_pushdown.rs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review feedback: the three-arm comparison harness is a local tool, not a maintained benchmark. Co-Authored-By: Claude Fable 5 <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.
Motivation
TakeExecmaterializes rows through the v2 readers' point-lookup path (ReadBatchParams::Indices)with per-fragment reader opens driven on a single task. Reading the same rows through
FilteredReadExec's planned range reads is much faster. Take-100 benchmark (100 random rowaddresses per query, warm NVMe, heavy ~1KiB string column; harness kept out of the tree):
_rowid IN (...))Carrying a payload column (e.g.
_distance/_score) through the row-stream read costs ~2% —the capability a row-set input cannot express.
Design: one I/O node over a row source
FilteredReadExecstays a single, general I/O node:Only the row source — who names the rows — varies, and the differences between sources are
inherent to what they are, not modes of the node:
IndexExprResultbatch (scalar index result,_rowid IN) — existing behavior, unchanged_rowid/_rowaddrcolumn_distance), preservedThe source kind is derived from the input plan's schema at construction — never configured
(the wire layouts are unmistakable; anything else must carry a row id/address column). Each
stream batch is converted to an in-memory
IndexExprResultand read through the sameplan_scan->plan_to_scoped_fragments->read_fragmentpipeline as a set; the readcolumns are then aligned (hash on the key column, O(N)) and merged back into the batch —
including nested-struct merges, via the same
calculate_output_schema+merge_with_schemacontract as
TakeExec. Streaming batch-by-batch: no input draining, same memory profile asTakeExec.Call sites:
Scanner::take()and merge_insert's indexed take now plan takes asFilteredReadExecon the v2 storage format. Plan text showsLanceRead: ..., projection=[...], source=stream(_rowid)whereTake: columns=...appearedbefore.
Design notes (from review)
read_ranges_tasksis a stub that errors ("Attempt to perform FilteredRead on v1 files") —FilteredReadExecphysically cannot read v1 files. v1 datasets keep bit-for-bit yesterday'sbehavior; TakeExec becomes deletable when v1 read support is dropped. The invariant is
commented at both swap sites.
input plan, not fragment metadata. It IS implementable without column I/O (stream the
input; per batch, count rows whose id is in the live-row set built from fragment metadata +
deletion vectors — per row, so duplicates count); deferred because no plan shape places a
row-stream read under a COUNT today (count plans request no columns, so no read node is
built). Recipe recorded in count_pushdown.rs.
(TakeExec itself sorts and inverse-permutes); duplicates and stable-row-id address order
make request-order reads impossible. The alignment is O(N) per batch and ~2% of query time.
Behavior change
Input rows whose row id/address no longer exists (stale index results pointing at deleted
rows) are now silently dropped on non-stable-row-id datasets, where
TakeExecfailed with arow-count mismatch error. This matches the existing stable-row-id behavior and FTS
deleted-row expectations.
Follow-ups (out of scope, marked in code)
filtered_read_exec_to_protoreturnsnot_supported).NULL
_rowidmemtable rows).cache for many-batch reads.
Testing
row ids, stale-id drops, construction errors,
with_new_children, execution without atokio runtime.
lancelib suite,--features slow_testsquery integration suite (incl. FTSstale-rowid),
lance-namespace-impls; plan-text assertions updated.🤖 Generated with Claude Code