Repository navigation
Plan a projection of fields as one pack of column plans - #10858
Draft
joseph-isaacs wants to merge 2 commits into
Draft
joseph-isaacs wants to merge 2 commits into
joseph-isaacs wants to merge 2 commits into
Conversation
…s as column packs The scan half of the DuckDB V2 work in #10833, split out so it can be reviewed on its own. Nothing here changes a driver; the DuckDB side follows in a separate PR. - `ChunkPlanner::with_chunks` takes chunks a driver cuts itself, and `ChunkPlanner::inline` runs a part's chunks as its children so they share one IO session. - `PreparedScan::chunk_rows` and `PreparedScan::sparse_selection` expose the scan's chunk size and whether it selects at most 1 row in 1024. - `SegmentScanIo` shares a segment's read across a scan's IO sessions, holding reads weakly, and retains them for the scan's lifetime under a sparse selection. - `EvalPackRule` plans a pack of field paths as one `PackPlan` of column plans, and `ExpressionPackRule` steps a field path into its field. - The row index compiles as a lazy sequence, so a sparse mask produces only its rows. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UeniBCK9ajvBLbjtkwq3cn Signed-off-by: Claude <noreply@anthropic.com>
They are only used by the DuckDB V2 driver, so they move to its own PR, leaving this one with the projection planning changes only. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UeniBCK9ajvBLbjtkwq3cn Signed-off-by: Claude <noreply@anthropic.com>
Merging this PR will degrade performance by 0.42%3 warnings, 1 note
Warning Please fix the performance issues or acknowledge them on CodSpeed. Performance Changes
Tip Investigate this regression by commenting Comparing 
Footnotes
|
joseph-isaacs
added
the
action/bench-sql
github-actions
Bot
removed
the
action/bench-sql
Contributor
Polar Signals Profiling ResultsLatest Run
Powered by Polar Signals Cloud |
Contributor
Benchmarks: PolarSignals Profiling 📖Commits: PR datafusion / vortex-file-compressed / ns (1.002x ➖, 0↑ 0↓)
No file size changes detected. |
Contributor
Benchmarks: FineWeb NVMe 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (1.033x ➖, 0↑ 2↓)
datafusion / parquet / ns (1.000x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (0.992x ➖, 0↑ 0↓)
duckdb / parquet / ns (1.008x ➖, 0↑ 0↓)
No file size changes detected. |
Contributor
Benchmarks: TPC-H SF=1 on NVME 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (0.958x ➖, 1↑ 0↓)
datafusion / parquet / ns (1.002x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (0.983x ➖, 1↑ 0↓)
duckdb / parquet / ns (0.999x ➖, 0↑ 0↓)
No file size changes detected. |
Contributor
Benchmarks: TPC-H SF=10 on NVME 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (0.981x ➖, 0↑ 0↓)
datafusion / parquet / ns (0.996x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (0.991x ➖, 0↑ 0↓)
duckdb / parquet / ns (0.999x ➖, 0↑ 0↓)
No file size changes detected. |
Contributor
Benchmarks: TPC-DS SF=1 on NVME 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (1.003x ➖, 0↑ 1↓)
datafusion / parquet / ns (0.999x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (1.001x ➖, 0↑ 0↓)
duckdb / parquet / ns (1.000x ➖, 0↑ 0↓)
No file size changes detected. |
Contributor
Benchmarks: Clickbench on NVME 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (0.998x ➖, 0↑ 0↓)
datafusion / parquet / ns (0.997x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (0.993x ➖, 0↑ 0↓)
duckdb / parquet / ns (0.991x ➖, 0↑ 0↓)
No file size changes detected. |
Contributor
Benchmarks: TPC-H SF=1 on S3 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (1.197x ➖, 0↑ 6↓)
datafusion / parquet / ns (1.090x ➖, 0↑ 1↓)
duckdb / vortex-file-compressed / ns (1.149x ➖, 0↑ 3↓)
duckdb / parquet / ns (1.108x ➖, 0↑ 2↓)
|
This branch has not been deployed
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
Split out of #10833, together with the DuckDB V2 driver changes on
ji/duckdb-v2-scan-driver. The two PRs touch no common files and can merge in either order.This changes how the V2 physical planner handles projections. The motivation is DuckDB's late-materialization row-id fetch in ClickBench q23, which projects 105 columns plus the row index for a few rows. That projection was packed, renamed and packed again for every chunk, comparing the wide dtypes deeply each time, and the row index was built for the whole chunk. The change applies to every V2 scan, including the DataFusion and DuckDB TPC-H scans already on V2.
Changes
PackPlan(vortex-layout/src/plan/physical/plans/eval.rs,pack.rs).EvalPackRulelifts a pack whose fields are all field paths into aPackchild, stepping each path into its column's plan.ExpressionPackRulesteps a field path into its field without annotating every field.plans/row_idx.rs,layouts/row_idx/mod.rs), as the V1 row-index reader does. A sparse mask then produces only its selected rows.field_projection_with_row_idx_packs_column_plansinplan/physical/tests.rscovers both.The branch has two commits. The first carried the whole
vortex-layouthalf of #10833. The second moves the scan IO and chunk planner changes to the DuckDB driver PR. The net diff is the five planning files.Checks
Not yet run on this branch. On #10833,
cargo test -p vortex-layout --libpassed, DataFusion TPC-H on V2 read −3.9% at SF1 and −1.8% at SF10, and clippy and fmt were clean.Generated by Claude Code