Repository navigation
Conversation
github-actions
Bot
added
documentation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #26273 +/- ##
==========================================
- Coverage 82.89% 82.82% -0.08%
==========================================
Files 1154 1154
Lines 457630 458230 +600
Branches 457630 458230 +600
==========================================
+ Hits 379370 379543 +173
- Misses 55256 55647 +391
- Partials 23004 23040 +36 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Thank you for opening this pull request! Reviewer note: cargo-semver-checks reported the current version number is not SemVer-compatible with the changes in this pull request (compared against the base branch). Details |
adriangb
force-pushed
the
mcte-proto
branch
2 times, most recently
from
October 16, 2026 03:46
e711398 to
12d61e5
Compare
adriangb
force-pushed
the
mcte-proto
branch
2 times, most recently
from
October 16, 2026 16:57
00e49a8 to
0a0fa38
Compare
Replace the `MaterializedCte` and `MaterializedCteScan` extension nodes with the variants `LogicalPlan::MaterializedCte` and `LogicalPlan::MaterializedCteScan`. Each rule now states what it does with the nodes in its `match`, and the downcasts are removed from `PushDownFilter`, `PullUpCorrelatedExpr`, `head_output_expr` and the physical planner. `MaterializedCte::try_new` checks that the scans have the schema of the body, and `MaterializedCteScan::find_in` finds the scans of one CTE, also in subquery expressions. The unparser, the Substrait producer and the protobuf serializer return a "not implemented" error for the two variants. Plans and results do not change. Closes apache#26238. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Add protobuf encodings for LogicalPlan::MaterializedCte, LogicalPlan::MaterializedCteScan, MaterializedCteExec and MaterializedCteScanExec. The physical encoding carries only the id of the CTE. The decode context holds one buffer for each id, so a decoded scan and its MaterializedCteExec share the buffer wherever they are in the plan, also when the scan is in a scalar subquery. Decoded buffers get new ids, because the ids of another process can be equal to local ids. A decoded plan fragment that has a scan without its MaterializedCteExec fails with a clear error when the scan is executed. Closes apache#26231. Co-Authored-By: Claude Opus 5.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
2 participants
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.
Which issue does this PR close?
Stacked on #26269. Only the last commit is new.
Rationale for this change
A plan that has a materialized CTE cannot be sent to another process.
Before this PR, the two calls return an error. After this PR, the two calls succeed. The decoded physical plan runs the body of the CTE one time, and all the scans read its buffer.
What changes are included in this PR?
MaterializedCteNode,MaterializedCteScanNode,MaterializedCteExecNodeandMaterializedCteScanExecNode.PhysicalPlanDecodeContextholds aDecodedMaterializedCteBuffersregistry. Each decoded node asks the registry for the buffer of its id. Thus a scan and itsMaterializedCteExecshare one buffer in any decode order, also when the scan is in a scalar subquery and is not below theMaterializedCteExec.MaterializedCteId::next(), because an id from another process can be equal to a local id.MaterializedCteExeccan be decoded. It fails when the scan is executed, with an error that says that the body and the scans must be in one plan and run in one process.Known limits:
PhysicalPlanDecodeContexthas a separate registry. A CTE that is split across such a boundary fails with the error above.projectionon the scan) and Push filters that all references share into the body of a materialized CTE #26239 (pushed_filter) add fields that this PR does not encode. The PR that merges second must add them.Are these changes tested?
Yes. Physical round trip of a CTE with two scans and of a CTE with a scan in a scalar subquery; each test runs the decoded plan and checks the result and that the body ran one time. A test for a scan without its
MaterializedCteExec. Logical round trip of the same two queries, before and after the optimizer. The error for two decodedMaterializedCteExecnodes with one id has no test.Are there any user-facing changes?
Yes. Plans with
WITH ... AS MATERIALIZEDcan be serialized withdatafusion-proto. API additions:MaterializedCteId::from_u64,MaterializedCteBuffer::name,DecodedMaterializedCteBuffers,try_from_protoon the two physical nodes, andExecutionPlanDecodeCtx::materialized_cte_buffer. The#[doc(hidden)]traitExecutionPlanDecodehas a new required method.