Fix/orca use after free having decorrelation#1620
Open
yjhjstz wants to merge 2 commits intoapache:mainfrom
Open
Fix/orca use after free having decorrelation#1620yjhjstz wants to merge 2 commits intoapache:mainfrom
yjhjstz wants to merge 2 commits intoapache:mainfrom
Conversation
yjhjstz
force-pushed
the
fix/orca-use-after-free-having-decorrelation
branch
from
69a2a53 to
601eff6
Compare
Guard pfree/list_free calls with pointer-equality checks to avoid freeing live nodes when flatten_join_alias_vars returns the same pointer unchanged (e.g., outer-reference Vars with varlevelsup > 0). The unconditional pfree(havingQual) freed the Var node, whose memory was later reused by palloc for a T_List. copyObjectImpl then copied the wrong node type into havingQual, causing ORCA to encounter an unexpected RangeTblEntry and fall back to the Postgres planner. Applies the same guard pattern to all six fields: targetList, returningList, havingQual, scatterClause, limitOffset, limitCount. Reported-in: apache#1618
yjhjstz
force-pushed
the
fix/orca-use-after-free-having-decorrelation
branch
from
601eff6 to
d50a94e
Compare
There was a problem hiding this comment.
Pull request overview
Fixes a use-after-free in query normalization (flatten_join_alias_var_optimizer) that could corrupt havingQual and trigger non-deterministic GPORCA fallback, and adjusts GPORCA scalar-subquery handling to preserve correct NULL-vs-0 semantics for correlated GROUP BY () HAVING <outer_ref> patterns.
Changes:
- Prevent freeing
havingQual/lists/nodes whenflatten_join_alias_vars()returns the original pointer (avoids use-after-free). - Detect correlated-HAVING-above-empty-GBAgg patterns in GPORCA and force/adjust handling to preserve correct NULL semantics.
- Update regression expected output to reflect GPORCA planning (no fallback) for the affected query shape.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/test/regress/expected/groupingsets_optimizer.out | Updates expected EXPLAIN output now that GPORCA handles the query (no Postgres fallback). |
| src/backend/optimizer/util/clauses.c | Adds pointer-equality guards before freeing nodes/lists returned by flatten_join_alias_vars() (fixes UAF). |
| src/backend/gporca/libgpopt/src/xforms/CSubqueryHandler.cpp | Adds detection of correlated HAVING over GROUP BY () to avoid incorrect COALESCE(count,0) decorrelation semantics. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Force correlated execution (SubPlan) for scalar subqueries with GROUP BY () and a correlated HAVING clause. Previously ORCA decorrelated such subqueries into Left Outer Join + COALESCE(count,0), which incorrectly returned 0 instead of NULL when the HAVING condition was false. Add FHasCorrelatedSelectAboveGbAgg() to detect the pattern where NormalizeHaving() has converted the HAVING clause into a CLogicalSelect with outer refs above a CLogicalGbAgg with empty grouping columns. When detected, set m_fCorrelatedExecution = true in Psd() to bypass the COALESCE decorrelation path. Update groupingsets_optimizer.out expected output to reflect the new ORCA SubPlan format instead of Postgres planner fallback. Reported-in: apache#1618
yjhjstz
force-pushed
the
fix/orca-use-after-free-having-decorrelation
branch
from
d50a94e to
fa35d74
Compare
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
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.
Fixes #1618
Root Cause
use-after-free (clauses.c): pfree(havingQual) freed the v.c Var node unconditionally. The freed memory was reused by palloc for a T_List (nodeTag=596). When copyObjectImpl later read havingQual, it copied the wrong node type, causing ORCA to encounter an unexpected RangeTblEntry and fall back to the Postgres planner — which masked the second bug.
ORCA decorrelation (CSubqueryHandler.cpp): After fixing the use-after-free, ORCA no longer fell back and attempted to decorrelate the subquery. It converted GROUP BY () HAVING <outer_ref> into Left Outer Join + COALESCE(count(*), 0), incorrectly returning 0 instead of NULL when the HAVING condition was false.
What does this PR do?
Type of Change
Breaking Changes
Test Plan
make installcheckmake -C src/test installcheck-cbdb-parallelImpact
Performance:
User-facing changes:
Dependencies:
Checklist
Additional Context
CI Skip Instructions