fix: explain analyze m-cte should work#19797
Conversation
2c6539a to
bf969bd
Compare
bf969bd to
e50e31f
Compare
Capture materialized CTE producer plans/profiles for EXPLAIN and EXPLAIN ANALYZE, keep executor-local profile ids isolated, and attach recursive CTE step profiles back under the recursive UnionAll scope.
e50e31f to
8cb7795
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8cb7795e96
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| ctx.get_function_context()?, | ||
| ctx.get_settings(), | ||
| ctx.clone(), | ||
| ); | ||
| pipeline.finalize(&plan)? |
There was a problem hiding this comment.
Restore distributed pipeline build for recursive CTE steps
Building recursive-step pipelines with PipelineBuilder::finalize(&plan) bypasses the distributed-plan path that build_query_pipeline_without_render_result_set previously handled. If the recursive branch contains Exchange nodes (common on cluster/warehouse deployments), this path no longer performs fragment/action setup and can fail when executing distributed recursive CTE queries. Please route this code back through the distributed-aware builder (or equivalent logic) while keeping the profile-scope guard.
Useful? React with 👍 / 👎.
| Ok(self | ||
| .ctx | ||
| .get_query_profiles() | ||
| .into_iter() | ||
| .filter(|x| x.id.is_some()) | ||
| .map(|x| (x.id.unwrap(), x)) | ||
| .collect::<HashMap<_, _>>()) | ||
| .get_query_profiles_for_execution(&profile_execution_id)) |
There was a problem hiding this comment.
Include captured CTE executions in graphical analyze profiles
execute_and_get_profiles now returns only get_query_profiles_for_execution(profile_execution_id), which excludes other execution namespaces such as materialized-CTE producer CTAS runs. Text explain output re-adds those via format_captured_cte_sections, but graphical output uses only this returned map, so EXPLAIN ANALYZE ... GRAPHICAL can silently miss materialized CTE producer profiles.
Useful? React with 👍 / 👎.
I hereby agree to the terms of the CLA available at: https://docs.databend.com/dev/policies/cla/
Summary
fix: m-cte profiles miss
Before this PR,
EXPLAIN ANALYZEcould lose or mis-attach profile information for nested query executions.The main problem was that profile collection used plan ids as if they were globally unique inside one
QueryContext. However, nested executors, materialized CTE producer CTAS runs, and recursive CTE step pipelines each emit executor-local plan ids. As a result:UnionAllscopeTests
Type of change
This change is