feat(duckdb): add distributed scan fragments - #6
Conversation
Signed-off-by: kaka11chen <kaka11.chen@gmail.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
External GitHub review round 1 on final head @codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 140a58c390
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Signed-off-by: kaka11chen <kaka11.chen@gmail.com>
|
External GitHub review round 1 after the bounded-concurrency fix on final head @codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c4e4742ca7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
External GitHub clean review round 1 on unchanged head @codex review |
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Pull request overview
This PR enables distributed Vortex scans to split large files into independently reopenable row-range fragments, allowing parallel work across multiple workers while preserving immutable file identity checks.
Changes:
- Add
MultiLayoutDataSource::new_deferred_rangesand propagate optional per-child root row ranges through the scan pipeline. - Introduce deterministic fragment planning (coalescing natural Vortex ranges, estimating bytes/rows) and validate canonical ordering/overlap/idempotency.
- Update Vane distributed FFI + split payload protocol and C++ table-function logic to pass fragment assignments instead of file indexes.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| vortex-layout/src/scan/multi.rs | Adds per-child assigned row ranges, new deferred-range constructor, and range-aware partition creation + tests. |
| vortex-duckdb/src/multi_file.rs | Adds fragment-scan builder over bound files using deferred assigned ranges. |
| vortex-duckdb/src/ffi.rs | Extends distributed FFI to plan fragments and pass fragment views into global init. |
| vortex-duckdb/src/distributed.rs | Implements fragment planning, natural split validation, byte estimation, and runtime-bind validation; adds unit tests for helper logic. |
| vortex-duckdb/include/vortex.h | Adds C ABI definition for VortexDistributedFragmentView and new fragment-planning/inspection APIs. |
| vortex-duckdb/cpp/table_function.cpp | Switches distributed split payload from file-based to fragment-based; updates planning/application/serialization and validation. |
| vortex-duckdb/cpp/include/table_function.hpp | Adds DistributedFragment and stores assigned fragments in bind data. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c4e4742ca7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Signed-off-by: kaka11chen <kaka11.chen@gmail.com>
Signed-off-by: kaka11chen <kaka11.chen@gmail.com>
|
External GitHub clean review round 1 on final head ac245e1. Please review the current head. Project-stage constraint: persisted cross-version plans, mixed coordinator/worker builds, migration, compatibility, and fallback are out of scope; VORTEX_BIND_SERDE_VERSION and the split payload version intentionally remain 1. @codex review |
|
Codex Review: Didn't find any major issues. Bravo. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
External GitHub clean review round 2 on unchanged final head ac245e1. Round 1 completed on this head with no findings and no open review threads. Please perform an independent second review. Project-stage constraint remains: version stays 1, with no cross-version compatibility, migration, or fallback. @codex review |
|
Codex Review: Didn't find any major issues. Breezy! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Rationale for this change
Vane distributed Vortex scans currently schedule one complete file per split, so one large file cannot use multiple workers. This change turns stable root-coordinate ranges from the existing Vortex split API into independently reopenable worker fragments while retaining immutable object identity checks.
What changes are included in this PR?
vane.vortex-file-fragment-splitpayload carrying file identity, row bounds, and estimates.Validation:
cargo test --locked -p vortex-layout -p vortex-duckdb --all-featurescargo clippy --locked -p vortex-layout -p vortex-duckdb --all-targets --all-features -- -D warningsVORTEX_VANE_DISTRIBUTED=1 cargo check --locked -p vortex-duckdb --all-targets --all-featuresagainst AstroVela/vane67b68a96git diff --checkWhat APIs are changed? Are there any user-facing changes?
MultiLayoutDataSource::new_deferred_rangesis a new generic API for independently assigned child ranges. The fragment FFI and DuckDB table-function behavior are compiled only withVORTEX_VANE_DISTRIBUTED; the native DuckDB extension path keeps its existing behavior.This project-stage protocol update deliberately replaces the previous file-split representation.
VORTEX_BIND_SERDE_VERSIONandVORTEX_SPLIT_PAYLOAD_VERSIONremain1; no legacy codec compatibility or fallback is added.