orca: fallback to Postgres optimizer on cross-slice replicated CTE Consumer#1719
Conversation
There was a problem hiding this comment.
Pull request overview
This PR prevents ORCA query hangs by detecting a specific unsafe plan shape—CTE Consumers executing in a different executor slice than their replicated-like CTE Producer—and forcing a fallback to the Postgres planner before DXL translation.
Changes:
- Add a physical-plan tree walker that records slice placement of replicated-like CTE Producers and CTE Consumers, and triggers an unsupported-feature exception when they’re cross-slice.
- Invoke the new detection at the start of Expr→DXL translation to force planner fallback instead of risking a hang.
- Add regression coverage for both the duplicate-hazard motion JOIN case and the scalar-subquery/shared-scan case.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/backend/gporca/libgpopt/include/gpopt/base/CUtils.h | Exposes new helpers/types for identifying problematic CTE Producer/Consumer placement. |
| src/backend/gporca/libgpopt/src/base/CUtils.cpp | Implements slice-tracking walker and cross-slice replicated-CTE Consumer detection. |
| src/backend/gporca/libgpopt/src/translate/CTranslatorExprToDXL.cpp | Calls the new detection early in translation and raises an unsupported-feature to force fallback. |
| src/test/regress/sql/qp_orca_fallback.sql | Adds a regression query for CTE Consumers under duplicate-hazard motion. |
| src/test/regress/expected/qp_orca_fallback.out | Captures expected planner output for the new regression case. |
| src/test/regress/expected/qp_orca_fallback_optimizer.out | Captures expected ORCA-fallback INFO/DETAIL plus Postgres plan output. |
| src/test/regress/sql/shared_scan.sql | Adds a scalar-subquery reproducer guarded by statement_timeout. |
| src/test/regress/expected/shared_scan.out | Captures expected results for the new shared-scan reproducer. |
| src/test/regress/expected/shared_scan_optimizer.out | Captures expected results for the ORCA-enabled variant of the shared-scan test. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
0fac296 to
b51a8fc
Compare
…nsumer. Inspired by greengage 51fe92e: before Expr->DXL translation, walk the physical tree and track which slice each CTE Producer and Consumer lives on. If a Consumer is on a different slice than its Producer and the Producer's distribution is replicated, force a fallback to the Postgres optimizer. The replicated filter is essential: ordinary cross-slice CTE plans (non-replicated Producer with Gather/Redistribute Consumer) are a normal ORCA pattern and must not trigger fallback. 51fe92e doesn't trigger when a CTE over a replicated table is referenced from a scalar subquery, so the query hangs. This commit replaces the single-point check with a whole-tree walker that catches both cases. Tests: shared_scan adds a scalar-subquery reproducer guarded by statement_timeout. qp_orca_fallback adds two cases over a replicated CTE: a scalar-subquery form that triggers the walker (the hang case 51fe92e missed -- fallback to Postgres), and the original 51fe92e JOIN form where ORCA emits a safe plan with a One-Time Filter (gp_execution_segment() = N) and the walker correctly stays silent (guards against false positives). (cherry picked from commit open-gpdb/gpdb@3a9aebf)
b51a8fc to
a5768d8
Compare
|
LGTM to fallback temporary, but maybe the best way to fix the root cause is fixing the Sequence::pdsRequired, in my opinion, this path is an illegal path which should be pruned by pdsRequired restrition. What's more, maybe we can add restriction between CCTEReq and CCTEMap by checking the motion worker number, only when they have the same motion workers, CCTEMap::FSatisfies will return true and we will get a legal path. Prefer to the first path, it's more formal than the second which looks like a remedial plan. |
Agree with considering it as a temporary solution. Besides, I have another open PR in open-gpdb where the Consumer adds its own local copy of the Producer's subtree, so orca is able to generate correct plan here. However, I noticed that it covers not all similar cases and because of this I started with fallback approach. |
Thanks for the suggestion. Agreed that pruning during search is cleaner than a post-hoc fallback, but both proposed variants only cover part of the cases. Sequence::PdsRequired already does the right thing on its own boundary: when the first child derives Replicated, it requests EdtReplicated on the second child. The issue is that the Motion which breaks slice alignment is enforced deeper inside the second child's subtree (typically a Gather under a scalar subquery) - so it lands in a different slice than the Producer, and that's not constrained by the spec Sequence sets at its own boundary. I'll try to fix it but I think it is a discussion for a different pr. CCTEMap::FSatisfies + motion worker count - at FSatisfies time we only know the kind of distribution from CDistributionSpec. Concrete slice ids don't exist yet - they get assigned later, when Motion nodes are enforced, and because of this we can't catch the original cross-slice problem. The current walker-based approach feels more general and reliable to me, because it catches the cross-slice problem regardless of which Motion produced it. |
leborchuk
left a comment
There was a problem hiding this comment.
LGTM, FFoundCrossSlice has nested loop (n^2 complexity), another approach is to use sorted arrays and merge two arrays, but the array sizes are small. No need to write complex logic here
What does this PR do?
Fixes a query hang in ORCA when a CTE over a replicated table is referenced
from a different executor slice than its Producer (e.g. from a scalar
subquery, or via duplicate-hazard motion in a JOIN).
Before Expr→DXL translation, the patch walks the physical tree and tracks
which slice each CTE Producer and Consumer lives on (slices are delimited
by Motion nodes). If any Consumer is on a different slice than its Producer
and the Producer's distribution is replicated-like
(StrictReplicated / TaintedReplicated / Universal), ORCA raises an
unsupported-feature exception and falls back to the Postgres planner.
The replicated filter is essential: ordinary cross-slice CTE plans
(non-replicated Producer with Gather/Redistribute Consumer) are a normal
ORCA pattern and must not trigger fallback.
Inspired by greengage 51fe92e, but extends it: 51fe92e's single-point check
doesn't trigger when a CTE over a replicated table is referenced from a
scalar subquery, so the query hangs. This PR replaces it with a whole-tree
walker that catches both cases.
Type of Change
Breaking Changes
None.
Test Plan
make installcheckmake -C src/test installcheck-cbdb-parallelTwo regression cases added:
qp_orca_fallback: original 51fe92e reproducer — JOIN over a CTEreferencing replicated tables with duplicate-hazard motion.
shared_scan: scalar-subquery reproducer where a CTE over a replicatedtable is referenced from multiple
(SELECT … FROM cte)expressions.Guarded by
statement_timeout = '15s'so the test fails fast if thehang regresses (40k rows in the driving table are needed to push ORCA
to the cross-slice plan — fewer rows would make the test silently pass
without the fix).
Impact
Performance: Negligible. The added walker runs once per query, only
over the physical tree, and only when CTE Producers/Consumers are present.
The set of queries that newly fall back to the Postgres planner is narrow
(replicated CTE Producer with a cross-slice Consumer) — these queries
previously hung, so any plan is an improvement.
User-facing changes: Queries matching the pattern above now complete
instead of hanging. Users will see
INFO: GPORCA failed to produce a plan, falling back to planner/DETAIL: Feature not supported: CTE Consumer placed on a different slice than its replicated Producerin those cases.Dependencies: None.
Checklist
Additional Context
Cherry-picked from open-gpdb/gpdb#375 (commit
3a9aebfe3afb27dd060c16b5cfa7932e7a2ed43f).Files changed:
src/backend/gporca/libgpopt/include/gpopt/base/CUtils.hsrc/backend/gporca/libgpopt/src/base/CUtils.cppsrc/backend/gporca/libgpopt/src/translate/CTranslatorExprToDXL.cppsrc/test/regress/{sql,expected}/qp_orca_fallback{,.out,_optimizer.out}src/test/regress/{sql,expected}/shared_scan{,.out,_optimizer.out}