You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudberry.apache.org by "zhangwenchao-123 (via GitHub)" <gi...@apache.org> on 2026/07/23 06:32:42 UTC
[PR] Fix ORCA plan param bitmaps and Material rescan shielding [cloudberry]
zhangwenchao-123 opened a new pull request, #1860:
URL: https://github.com/apache/cloudberry/pull/1860
ORCA-translated plans get their extParam/allParam bitmapsets from SetParamIds(), called piecemeal by every translator function. These bitmaps are what rescan correctness hangs on: the executor propagates chgParam to a child only if the child's allParam contains the changed param (UpdateChangedParamSet()), and a Material node relies on that to discard its tuplestore when a param of an enclosing SubPlan changes. Any translator path that misses the call leaves a node with an empty bitmap, which silently breaks the chgParam chain below it: a Material sitting above a subtree that consumes a correlated param then replays the first outer row's result for every subsequent row. GPDB 7.5.x shipped exactly this class of wrong-results regression for correlated subqueries computing UNNEST over an outer column (fixed in 7.6.0).
Two hardenings:
1. Replace the per-translator SetParamIds() calls with one authoritative pass in the ORCA post-processing (orca.c), run over the final shape of the plan after all plan mutations. Every plan node, in the main tree and in every subplan, now gets its bitmaps computed in one place, so a future translator path cannot reintroduce the wrong-results hazard by forgetting a call.
The pass counts only PARAM_EXEC params, matching the regular planner's finalize_primnode(): PARAM_EXTERN paramids live in a separate numbering space (the client's $n) and never change during execution, so including them would alias unrelated exec params and trigger spurious rescans of materialized subtrees.
2. Set Material's cdb_shield_child_from_rescans only when the subtree below actually contains a Motion. That flag exists to protect Motions, which cannot be rescanned, from rescan and squelch (see the planner-side uses in pathnode.c); the ORCA translator set it unconditionally on every Material. For a Motion-free subtree the shield adds no benefit, and if the subtree consumes params of an enclosing SubPlan it needlessly makes the cached result's invalidation depend solely on the param bitmaps being right.
The ORCA optimizer model itself is sound and unchanged: a Spool over a subtree with outer refs requests Rescannable from its children (CPhysicalSpool::PrsRequired), i.e. it already assumes the executor rebuilds the spooled result whenever the correlated params change.
The new orca_material_rescan regression test pins the vulnerable plan shape - a correlated SubPlan whose nestloop inner side is a Material over a Motion-free ProjectSet computing UNNEST over an outer column - and verifies per-row results. Its tables are left unanalyzed on purpose: with default cardinalities ORCA places the ProjectSet under the inner-side Material, which is the shape this fix protects.
<!-- Thank you for your contribution to Apache Cloudberry (Incubating)! -->
Fixes #ISSUE_Number
### What does this PR do?
<!-- Brief overview of the changes, including any major features or fixes -->
### Type of Change
- [ ] Bug fix (non-breaking change)
- [ ] New feature (non-breaking change)
- [ ] Breaking change (fix or feature with breaking changes)
- [ ] Documentation update
### Breaking Changes
<!-- Remove if not applicable. If yes, explain impact and migration path -->
### Test Plan
<!-- How did you test these changes? -->
- [ ] Unit tests added/updated
- [ ] Integration tests added/updated
- [ ] Passed `make installcheck`
- [ ] Passed `make -C src/test installcheck-cbdb-parallel`
### Impact
<!-- Remove sections that don't apply -->
**Performance:**
<!-- Any performance implications? -->
**User-facing changes:**
<!-- Any changes visible to users? -->
**Dependencies:**
<!-- New dependencies or version changes? -->
### Checklist
- [ ] Followed [contribution guide](https://cloudberry.apache.org/contribute/code)
- [ ] Added/updated documentation
- [ ] Reviewed code for security implications
- [ ] This PR contains AI-assisted code generation
- [ ] Requested review from [cloudberry committers](https://github.com/orgs/apache/teams/cloudberry-committers)
### Additional Context
<!-- Any other information that would help reviewers? Remove if none -->
### CI Skip Instructions
<!--
To skip CI builds, add the appropriate CI skip identifier to your PR title.
The identifier must:
- Be in square brackets []
- Include the word "ci" and either "skip" or "no"
- Only use for documentation-only changes or when absolutely necessary
-->
---
<!-- Join our community:
- Mailing list: [dev@cloudberry.apache.org](https://lists.apache.org/list.html?dev@cloudberry.apache.org) (subscribe: dev-subscribe@cloudberry.apache.org)
- Discussions: https://github.com/apache/cloudberry/discussions -->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For additional commands, e-mail: commits-help@cloudberry.apache.org