Repository navigation
Conversation
leborchuk
approved these changes
Oct 6, 2026
leborchuk
left a comment
Contributor
There was a problem hiding this comment.
LGTM, gp_dist_random is very special function, it's hard for me to make up a counter-example. But maybe it's not needed at all. Let's fix all new cases and make sure old one works as expected.
The UNION ALL pull-up path does not respect forceDistRandom, unlike ordinary subquery pull-up. This can bypass the planning logic that enforces execution on segments and produce incorrect results. Prevent UNION ALL pull-up when forceDistRandom is set, preserving the required execution location through the existing subquery planning path.
RangeTblEntry equality ignores forceDistRandom, allowing ordinary and forced-distributed scans to compare equal. OR simplification can then eliminate a semantically different EXISTS subquery and produce a result that depends on operand order. Compare forceDistRandom in _equalRangeTblEntry() so expression equality preserves the required execution location. Add regression coverage for ordinary and distributed EXISTS expressions, both OR orders, and AND controls.
Alena0704
force-pushed
the
fix-gp-dist-random-union-all-rel2
branch
from
October 6, 2026 19:15
dd5c348 to
2ceab2e
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.
What does this PR do?
Fix two cases where the PostgreSQL planner fails to account for
RangeTblEntry.forceDistRandom, the flag used bygp_dist_random()to requestexecution on segments. Execution location affects query semantics: ignoring
this flag can change the returned rows or the truth value of a condition.
The two commits address separate paths:
forceDistRandomis set, so the existing subquery planning path can enforce segment execution.
forceDistRandomso ordinary andforced-distributed scans are not treated as equivalent during expression
simplification.
Both reproductions below use only built-in objects. Run each block in one
session connected to a coordinator with multiple primary segments. The shown
results are from a three-segment cluster with segment IDs 0, 1 and 2;
gp_execution_segment() = -1identifies execution on the coordinator.Case 1: UNION ALL pull-up loses segment execution
Ordinary subquery pull-up already respects
forceDistRandom, but the simpleUNION ALL pull-up path does not. Flattening the view into an append relation
bypasses the subquery planning logic that enforces execution on segments.
Before the fix, the plan is an
Appendwith twoSeq Scan on gp_idchildrenand no Motion. Both branches execute on the coordinator:
Plan before the fix:
There is no Motion: both branches execute on the coordinator.
Plan after the fix:
The Gather Motion collects the output of both branches from all three primary
segments. After the fix, each branch executes once on every primary segment:
Fix: add
!rte->forceDistRandomto the UNION ALL pull-up condition inpull_up_subqueries_recurse(). Keeping the subquery intact letsset_subquery_pathlist()preserve the required execution location. Subquerieswithout this flag remain eligible for pull-up.
Case 2: RTE equality allows an incorrect OR simplification
_equalRangeTblEntry()does not compareforceDistRandom. Consequently, anordinary scan and a forced-distributed scan can compare equal even though they
execute in different places. OR simplification can then remove a semantically
different EXISTS subquery.
In this example, the first EXISTS is false on the coordinator and the second
is true on the segments. Their OR must be true.
ONLYmakes the ordinary RTE'sinheritance flag match the RTE created by
gp_dist_random(), exposing themissing comparison of
forceDistRandom.Before the fix, only the coordinator InitPlan survives:
The executed query incorrectly returns no rows:
Reversing the two EXISTS operands returns one row on the same affected build.
The result therefore depends on operand order. This case still fails with
only the UNION ALL pull-up fix applied.
After the equality fix, both InitPlans remain:
The query returns the correct result in either OR order:
Fix: add
COMPARE_SCALAR_FIELD(forceDistRandom)to_equalRangeTblEntry(). Query-tree equality must distinguish scans withdifferent execution requirements.
Type of Change
Test Plan
Extended the existing
gpdistregression test and both expected-outputvariants. The new blocks select
optimizer = offwithin transactions and cover:per segment; ordinary view access; a nested filtered view; and an empty result.
orders, and both AND orders as controls.
Impact
The changes preserve the execution semantics requested by
gp_dist_random().UNION ALL subqueries with
forceDistRandomretain their subquery boundary;expression equality distinguishes ordinary and forced-distributed scans.
Checklist
Checklist