fix(query): align profiles with physical plan scope#19963
Conversation
dqhl76
changed the title
dqhl76
force-pushed
the
fix-m-cte-profiles
branch
2 times, most recently
from
58d8f88 to
18c87e0
Compare
dqhl76
force-pushed
the
fix-m-cte-profiles
branch
from
18c87e0 to
24af834
Compare
|
@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: a87a8b9267
ℹ️ 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".
| pub fn add(&self, profiles: &HashMap<u32, PlanProfile>) { | ||
| let mut merged_profiles = self.profiles.lock(); | ||
|
|
||
| for query_profile in profiles.values() { | ||
| match merged_profiles.entry((query_profile.group_id, query_profile.id)) { |
There was a problem hiding this comment.
This store keys by (group_id, plan_id), but add still accepts batches keyed only by u32; the batches produced by RunningGraph::fetch_profiling have already merged all nodes with the same plan_id via plans_profile.entry(*plan_id) before group_id is considered. When one executor contains multiple physical plan trees with overlapping ids (for example source pipelines built from separate plans), the first group's PlanProfile absorbs the other group's statistics, so get_by_group(main_group_id) can return overcounted main profiles while the other group disappears. The executor-side aggregation needs to key by group as well, or this new grouping cannot recover the separated profiles here.
Useful? React with 👍 / 👎.
2 participants
I hereby agree to the terms of the CLA available at: https://docs.databend.com/dev/policies/cla/
Summary
Fix query profile plan id collisions when a query uses nested executors, such as materialized CTE execution.
Tests
Type of change
This change is