Skip to content

Honor forceDistRandom in subquery pull-up and equality - #2084

Open
Alena0704 wants to merge 2 commits into
apache:REL_2_STABLEfrom
Alena0704:fix-gp-dist-random-union-all-rel2
Open

Alena0704 wants to merge 2 commits into
apache:REL_2_STABLEfrom
Alena0704:fix-gp-dist-random-union-all-rel2

Conversation

@Alena0704

@Alena0704 Alena0704 commented Oct 6, 2026 •

Copy link
Copy Markdown
Collaborator

What does this PR do?

Fix two cases where the PostgreSQL planner fails to account for
RangeTblEntry.forceDistRandom, the flag used by gp_dist_random() to request
execution 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:

  1. UNION ALL pull-up: preserve the subquery boundary when forceDistRandom
    is set, so the existing subquery planning path can enforce segment execution.
  2. Range table entry equality: compare forceDistRandom so ordinary and
    forced-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() = -1 identifies execution on the coordinator.

Case 1: UNION ALL pull-up loses segment execution

Ordinary subquery pull-up already respects forceDistRandom, but the simple
UNION ALL pull-up path does not. Flattening the view into an append relation
bypasses the subquery planning logic that enforces execution on segments.

CREATE TEMP VIEW gdr_union_repro AS
  SELECT gp_execution_segment() AS seg, 1 AS branch FROM gp_id
  UNION ALL
  SELECT gp_execution_segment(), 2 FROM gp_id;

EXPLAIN (COSTS OFF)
SELECT * FROM gp_dist_random('gdr_union_repro');

SELECT * FROM gp_dist_random('gdr_union_repro') ORDER BY branch, seg;

Before the fix, the plan is an Append with two Seq Scan on gp_id children
and no Motion. Both branches execute on the coordinator:

Plan before the fix:

Append
  ->  Seq Scan on gp_id
  ->  Seq Scan on gp_id gp_id_1
Optimizer: Postgres query optimizer

There is no Motion: both branches execute on the coordinator.

 seg | branch
-----+--------
  -1 |      1
  -1 |      2
(2 rows)

Plan after the fix:

Gather Motion 3:1  (slice1; segments: 3)
  ->  Append
        ->  Seq Scan on gp_id
        ->  Seq Scan on gp_id gp_id_1
Optimizer: Postgres query optimizer

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:

 seg | branch
-----+--------
   0 |      1
   1 |      1
   2 |      1
   0 |      2
   1 |      2
   2 |      2
(6 rows)

Fix: add !rte->forceDistRandom to the UNION ALL pull-up condition in
pull_up_subqueries_recurse(). Keeping the subquery intact lets
set_subquery_pathlist() preserve the required execution location. Subqueries
without this flag remain eligible for pull-up.

Case 2: RTE equality allows an incorrect OR simplification

_equalRangeTblEntry() does not compare forceDistRandom. Consequently, an
ordinary 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. ONLY makes the ordinary RTE's
inheritance flag match the RTE created by gp_dist_random(), exposing the
missing comparison of forceDistRandom.

EXPLAIN (COSTS OFF)
SELECT 1 AS result
WHERE EXISTS (
  SELECT 1 FROM ONLY gp_id WHERE gp_execution_segment() >= 0
)
OR EXISTS (
  SELECT 1 FROM gp_dist_random('gp_id') WHERE gp_execution_segment() >= 0
);

SELECT 1 AS result
WHERE EXISTS (
  SELECT 1 FROM ONLY gp_id WHERE gp_execution_segment() >= 0
)
OR EXISTS (
  SELECT 1 FROM gp_dist_random('gp_id') WHERE gp_execution_segment() >= 0
);

Before the fix, only the coordinator InitPlan survives:

Result
  One-Time Filter: $0
  InitPlan 1 (returns $0)
    ->  Seq Scan on gp_id
          Filter: (gp_execution_segment() >= 0)
Optimizer: Postgres query optimizer

The executed query incorrectly returns no rows:

 result
--------
(0 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:

Result
  One-Time Filter: ($0 OR $1)
  InitPlan 1 (returns $0)  (slice1)
    ->  Seq Scan on gp_id
          Filter: (gp_execution_segment() >= 0)
  InitPlan 2 (returns $1)  (slice2)
    ->  Gather Motion 3:1  (slice3; segments: 3)
          ->  Seq Scan on gp_id gp_id_1
                Filter: (gp_execution_segment() >= 0)
Optimizer: Postgres query optimizer

The query returns the correct result in either OR order:

 result
--------
      1
(1 row)

Fix: add COMPARE_SCALAR_FIELD(forceDistRandom) to
_equalRangeTblEntry(). Query-tree equality must distinguish scans with
different execution requirements.

Type of Change

  • Bug fix (non-breaking change)

Test Plan

Extended the existing gpdist regression test and both expected-output
variants. The new blocks select optimizer = off within transactions and cover:

  • UNION ALL execution on every primary segment, with exactly one row per branch
    per segment; ordinary view access; a nested filtered view; and an empty result.
  • Ordinary and distributed EXISTS expressions independently, both OR operand
    orders, and both AND orders as controls.

Impact

The changes preserve the execution semantics requested by gp_dist_random().
UNION ALL subqueries with forceDistRandom retain their subquery boundary;
expression equality distinguishes ordinary and forced-distributed scans.

Checklist

  • Regression coverage added
  • This PR contains AI-assisted code generation
  • Full regression suite passed

Checklist

@Alena0704 Alena0704 added the type: Bug Something isn't working label Oct 6, 2026
@Alena0704
Alena0704 requested a review from leborchuk October 6, 2026 13:30

@leborchuk leborchuk left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Alena0704 force-pushed the fix-gp-dist-random-union-all-rel2 branch from dd5c348 to 2ceab2e Compare October 6, 2026 19:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: Bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants