Repository navigation
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hi, @Deneby67 welcome!🎊 Thanks for taking the effort to make our project better! 🙌 Keep making such awesome contributions!
Member
|
Thanks for tracking this down. The fix is correct for the ProjectSet chain. It is one instance of a broader problem, though: In our downstream tree we fixed this class of bug once instead of per call site (patch below):
With it, both queries in this PR's test return the expected rows (checked on our tree), and The patch is against our tree, not apache main: the 0001-Fix-ORCA-plan-param-bitmaps-and-Material-rescan-shie.patchFrom 572559301d11726a8b593da75a48fab0c0937fbe Mon Sep 17 00:00:00 2001
From: Zhang Wenchao <zhangwenchao@apache.org>
Date: Tue, 14 Jul 2026 18:49:19 +0800
Subject: [PATCH] Fix ORCA plan param bitmaps and Material rescan shielding
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.
---
.../translate/CTranslatorDXLToPlStmt.cpp | 90 +++-------------
src/backend/optimizer/plan/orca.c | 71 ++++++++++++
.../gpopt/translate/CTranslatorDXLToPlStmt.h | 3 -
.../regress/expected/orca_material_rescan.out | 101 ++++++++++++++++++
src/test/regress/greenplum_schedule | 2 +-
src/test/regress/sql/orca_material_rescan.sql | 76 +++++++++++++
6 files changed, 262 insertions(+), 81 deletions(-)
create mode 100644 src/test/regress/expected/orca_material_rescan.out
create mode 100644 src/test/regress/sql/orca_material_rescan.sql
diff --git a/src/backend/gpopt/translate/CTranslatorDXLToPlStmt.cpp b/src/backend/gpopt/translate/CTranslatorDXLToPlStmt.cpp
index 0e15485513e..86d1eaf3f9d 100644
--- a/src/backend/gpopt/translate/CTranslatorDXLToPlStmt.cpp
+++ b/src/backend/gpopt/translate/CTranslatorDXLToPlStmt.cpp
@@ -620,34 +620,6 @@ CTranslatorDXLToPlStmt::TranslateDXLOperatorToPlan(
return plan;
}
-//---------------------------------------------------------------------------
-// @function:
-// CTranslatorDXLToPlStmt::SetParamIds
-//
-// @doc:
-// Set the bitmapset with the param_ids defined in the plan
-//
-//---------------------------------------------------------------------------
-void
-CTranslatorDXLToPlStmt::SetParamIds(Plan *plan)
-{
- List *params_node_list = gpdb::ExtractNodesPlan(
- plan, T_Param, true /* descend_into_subqueries */);
-
- ListCell *lc = nullptr;
-
- Bitmapset *bitmapset = nullptr;
-
- ForEach(lc, params_node_list)
- {
- Param *param = (Param *) lfirst(lc);
- bitmapset = gpdb::BmsAddMember(bitmapset, param->paramid);
- }
-
- plan->extParam = bitmapset;
- plan->allParam = bitmapset;
-}
-
List *
CTranslatorDXLToPlStmt::TranslatePartOids(IMdIdArray *parts, INT lockmode)
{
@@ -785,7 +757,6 @@ CTranslatorDXLToPlStmt::TranslateDXLTblScan(
// translate operator costs
TranslatePlanCosts(tbl_scan_dxlnode, plan);
- SetParamIds(plan);
return plan_return;
}
@@ -890,7 +861,6 @@ CTranslatorDXLToPlStmt::TranslateDXLParallelTblScan(
plan->plan_rows = ceil(plan->plan_rows / parallel_workers);
}
- SetParamIds(plan);
return plan_return;
}
@@ -1082,7 +1052,6 @@ CTranslatorDXLToPlStmt::TranslateDXLIndexScan(
* As of 8.4, the indexstrategy and indexsubtype fields are no longer
* available or needed in IndexScan. Ignore them.
*/
- SetParamIds(plan);
return (Plan *) index_scan;
}
@@ -1180,7 +1149,6 @@ CTranslatorDXLToPlStmt::TranslateDXLParallelIndexScan(
* As of 8.4, the indexstrategy and indexsubtype fields are no longer
* available or needed in IndexScan. Ignore them.
*/
- SetParamIds(plan);
// Adjust row count to per-worker statistics
if (parallel_workers > 1)
@@ -1332,7 +1300,6 @@ CTranslatorDXLToPlStmt::TranslateDXLIndexOnlyScan(
}
index_scan->indexqual = index_cond;
- SetParamIds(plan);
return (Plan *) index_scan;
}
@@ -1407,7 +1374,6 @@ CTranslatorDXLToPlStmt::TranslateDXLParallelIndexOnlyScan(
plan->plan_rows = ceil(plan->plan_rows / parallel_workers);
}
- SetParamIds(plan);
return (Plan *) index_scan;
}
@@ -1747,7 +1713,6 @@ CTranslatorDXLToPlStmt::TranslateDXLLimit(
limit->limitOffset = limit_offset;
}
- SetParamIds(plan);
// cleanup
child_contexts->Release();
@@ -2039,7 +2004,6 @@ CTranslatorDXLToPlStmt::TranslateDXLHashJoin(
plan->lefttree = left_plan;
plan->righttree = right_plan;
- SetParamIds(plan);
// cleanup
translation_context_arr_with_siblings->Release();
@@ -2313,7 +2277,6 @@ CTranslatorDXLToPlStmt::TranslateDXLParallelHashJoin(
plan->lefttree = left_plan;
plan->righttree = right_plan;
- SetParamIds(plan);
// Adjust row count to per-worker statistics for parallel execution
// Use plan->parallel (inherited from probe side) to ensure consistency
@@ -2415,7 +2378,6 @@ CTranslatorDXLToPlStmt::TranslateDXLTvf(
}
func_scan->functions = ListMake1(rtfunc);
- SetParamIds(plan);
return (Plan *) func_scan;
}
@@ -2788,7 +2750,6 @@ CTranslatorDXLToPlStmt::TranslateDXLNLJoin(
}
plan->lefttree = left_plan;
plan->righttree = right_plan;
- SetParamIds(plan);
// cleanup
translation_context_arr_with_siblings->Release();
@@ -2901,7 +2862,6 @@ CTranslatorDXLToPlStmt::TranslateDXLMergeJoin(
plan->lefttree = left_plan;
plan->righttree = right_plan;
- SetParamIds(plan);
merge_join->mergeFamilies =
(Oid *) gpdb::GPDBAlloc(sizeof(Oid) * num_join_conds);
@@ -3008,7 +2968,6 @@ CTranslatorDXLToPlStmt::TranslateDXLHash(
plan->qual = NIL;
hash->rescannable = false;
- SetParamIds(plan);
return (Plan *) hash;
}
@@ -3082,7 +3041,6 @@ CTranslatorDXLToPlStmt::TranslateDXLParallelHash(
plan->qual = NIL;
hash->rescannable = false;
- SetParamIds(plan);
return (Plan *) hash;
}
@@ -3398,7 +3356,6 @@ CTranslatorDXLToPlStmt::TranslateDXLMotion(
plan->plan_rows = ceil(plan->plan_rows / sendslice->parallel_workers);
}
- SetParamIds(plan);
return (Plan *) motion;
}
@@ -3531,7 +3488,6 @@ CTranslatorDXLToPlStmt::TranslateDXLRedistributeMotionToResultHashFilters(
plan->lefttree = child_plan;
- SetParamIds(plan);
Plan *child_result = (Plan *) result;
@@ -3584,7 +3540,6 @@ CTranslatorDXLToPlStmt::TranslateDXLRedistributeMotionToResultHashFilters(
plan->qual = NIL;
plan->lefttree = child_result;
- SetParamIds(plan);
return (Plan *) result;
}
@@ -3872,7 +3827,6 @@ CTranslatorDXLToPlStmt::TranslateDXLAgg(
}
}
- SetParamIds(plan);
// cleanup
child_contexts->Release();
@@ -4202,7 +4156,6 @@ CTranslatorDXLToPlStmt::TranslateDXLWindowAgg(
}
}
- SetParamIds(plan);
// cleanup
child_contexts->Release();
@@ -4406,7 +4359,6 @@ CTranslatorDXLToPlStmt::TranslateDXLWindowHashAgg(
}
}
- SetParamIds(plan);
// cleanup
child_contexts->Release();
@@ -4524,7 +4476,6 @@ CTranslatorDXLToPlStmt::TranslateDXLSort(
TranslateSortCols(sort_col_list_dxl, &child_context, sort->sortColIdx,
sort->sortOperators, sort->collations, sort->nullsFirst);
- SetParamIds(plan);
// cleanup
child_contexts->Release();
@@ -4664,7 +4615,6 @@ CTranslatorDXLToPlStmt::TranslateDXLPartitionTopK(
GPOS_WSZ_LIT("PartitionTopK N must be a positive integer"));
}
- SetParamIds(plan);
child_contexts->Release();
return (Plan *) topk_plan;
@@ -4783,7 +4733,6 @@ CTranslatorDXLToPlStmt::TranslateDXLProjectSet(const CDXLNode *result_dxlnode)
// translate operator costs
TranslatePlanCosts(result_dxlnode, plan);
- SetParamIds(plan);
return (Plan *) project_set;
}
@@ -5085,7 +5034,6 @@ CTranslatorDXLToPlStmt::TranslateDXLResult(
plan->qual = quals_list;
result->resconstantqual = (Node *) one_time_quals_list;
- SetParamIds(plan);
// Creating project set nodes plan tree
Plan *project_set_parent_plan = CreateProjectSetNodeTree(
@@ -5214,7 +5162,6 @@ CTranslatorDXLToPlStmt::TranslateDXLPartSelector(
partition_selector->part_prune_info = MakeNode(PartitionPruneInfo);
partition_selector->part_prune_info->prune_infos = prune_infos;
- SetParamIds(plan);
// cleanup
child_contexts->Release();
@@ -5383,7 +5330,6 @@ CTranslatorDXLToPlStmt::TranslateDXLAppend(
nullptr, // translate context for the base table
child_contexts, output_context);
- SetParamIds(plan);
// cleanup
child_contexts->Release();
@@ -5539,7 +5485,6 @@ CTranslatorDXLToPlStmt::TranslateDXLParallelAppend(
nullptr, // translate context for the base table
child_contexts, output_context);
- SetParamIds(plan);
// cleanup
child_contexts->Release();
@@ -5570,8 +5515,6 @@ CTranslatorDXLToPlStmt::TranslateDXLMaterialize(
CDXLPhysicalMaterialize::Cast(materialize_dxlnode->GetOperator());
materialize->cdb_strict = materialize_dxlop->IsEager();
- // ensure that executor actually materializes results
- materialize->cdb_shield_child_from_rescans = true;
// translate operator costs
TranslatePlanCosts(materialize_dxlnode, plan);
@@ -5601,7 +5544,19 @@ CTranslatorDXLToPlStmt::TranslateDXLMaterialize(
plan->lefttree = child_plan;
- SetParamIds(plan);
+ // Shield the child from rescans only if there is a Motion somewhere in
+ // the subtree: Motions cannot be rescanned, so the Material must then
+ // keep its tuplestore across rescans and squelching must not propagate
+ // below it. For a Motion-free subtree rely on normal executor
+ // semantics instead; in particular, if the subtree refers to exec
+ // params of an enclosing SubPlan, the materialized result must be
+ // discarded and rebuilt whenever those params change (chgParam), and
+ // shielding it would add no benefit while making that dependency
+ // fragile.
+ List *child_motions =
+ gpdb::ExtractNodesPlan(child_plan, T_Motion,
+ true /* descendIntoSubqueries */);
+ materialize->cdb_shield_child_from_rescans = (NIL != child_motions);
// cleanup
child_contexts->Release();
@@ -5661,7 +5616,6 @@ CTranslatorDXLToPlStmt::TranslateDXLCTEProducerToSharedScan(
plan->lefttree = child_plan;
plan->qual = NIL;
- SetParamIds(plan);
// cleanup
child_contexts->Release();
@@ -5747,7 +5701,6 @@ CTranslatorDXLToPlStmt::TranslateDXLParallelCTEProducerToParallelSharedScan(
plan->lefttree = child_plan;
plan->qual = NIL;
- SetParamIds(plan);
// cleanup
child_contexts->Release();
@@ -5840,7 +5793,6 @@ CTranslatorDXLToPlStmt::TranslateDXLCTEConsumerToSharedScan(
plan->qual = nullptr;
- SetParamIds(plan);
// DON'T REMOVE, if current consumer need projection, then we can direct add it.
// we still keep the path of projection in consumer
@@ -5953,7 +5905,6 @@ CTranslatorDXLToPlStmt::TranslateDXLParallelCTEConsumerToParallelSharedScan(
plan->qual = nullptr;
- SetParamIds(plan);
// DON'T REMOVE, if current consumer need projection, then we can direct add it.
// we still keep the path of projection in consumer
@@ -6019,7 +5970,6 @@ CTranslatorDXLToPlStmt::TranslateDXLSequence(
nullptr, // base table translation context
child_contexts, output_context);
- SetParamIds(plan);
// cleanup
child_contexts->Release();
@@ -6085,7 +6035,6 @@ CTranslatorDXLToPlStmt::TranslateDXLParallelSequence(
nullptr, // base table translation context
child_contexts, output_context);
- SetParamIds(plan);
// cleanup
child_contexts->Release();
@@ -6179,7 +6128,6 @@ CTranslatorDXLToPlStmt::TranslateDXLDynTblScan(
security_query_quals = gpdb::ListConcat(security_query_quals, query_quals);
plan->qual = security_query_quals;
- SetParamIds(plan);
return plan;
}
@@ -6265,7 +6213,6 @@ CTranslatorDXLToPlStmt::TranslateDXLDynIdxOnlyScan(
dyn_idx_only_scan->indexscan.indexqual = index_cond;
- SetParamIds(plan);
return (Plan *) dyn_idx_only_scan;
}
@@ -6346,7 +6293,6 @@ CTranslatorDXLToPlStmt::TranslateDXLDynIdxScan(
dyn_idx_only_scan->indexscan.indexqual = index_cond;
dyn_idx_only_scan->indexscan.indexqualorig = index_orig_cond;
- SetParamIds(plan);
return (Plan *) dyn_idx_only_scan;
}
@@ -6508,7 +6454,6 @@ CTranslatorDXLToPlStmt::TranslateDXLDynForeignScan(
// translate operator costs
TranslatePlanCosts(dyn_foreign_scan_dxlnode, plan);
- SetParamIds(plan);
return plan;
}
@@ -6670,7 +6615,6 @@ CTranslatorDXLToPlStmt::TranslateDXLDml(
result_plan->lefttree = child_plan;
result_plan->targetlist = dml_target_list;
- SetParamIds(result_plan);
if (m_cmd_type == CMD_UPDATE && isSplit)
{
@@ -6683,7 +6627,6 @@ CTranslatorDXLToPlStmt::TranslateDXLDml(
final_result->resconstantqual =
(Node *) gpdb::LAppend(NIL, gpdb::MakeBoolConst(true /*value*/, false /*isnull*/));
- SetParamIds(final_result_plan);
result = final_result;
result_plan = final_result_plan;
@@ -6713,7 +6656,6 @@ CTranslatorDXLToPlStmt::TranslateDXLDml(
plan->targetlist = NIL;
plan->plan_node_id = m_dxl_to_plstmt_context->GetNextPlanId();
- SetParamIds(plan);
if (m_is_tgt_tbl_distributed)
{
@@ -6963,7 +6905,6 @@ CTranslatorDXLToPlStmt::TranslateDXLSplit(
plan->lefttree = child_plan;
plan->plan_node_id = m_dxl_to_plstmt_context->GetNextPlanId();
- SetParamIds(plan);
// cleanup
child_contexts->Release();
@@ -7044,7 +6985,6 @@ CTranslatorDXLToPlStmt::TranslateDXLAssert(
GPOS_ASSERT(gpdb::ListLength(plan->qual) ==
gpdb::ListLength(assert_node->errmessage));
- SetParamIds(plan);
// cleanup
child_contexts->Release();
@@ -8128,7 +8068,6 @@ CTranslatorDXLToPlStmt::TranslateDXLCtas(
child_contexts, output_context);
SetVarTypMod(phy_ctas_dxlop, target_list);
- SetParamIds(plan);
// cleanup
child_contexts->Release();
@@ -8152,7 +8091,6 @@ CTranslatorDXLToPlStmt::TranslateDXLCtas(
result_plan->lefttree = plan;
result_plan->targetlist = target_list;
- SetParamIds(result_plan);
plan = (Plan *) result;
@@ -8421,7 +8359,6 @@ CTranslatorDXLToPlStmt::TranslateDXLBitmapTblScan(
bitmap_tbl_scan->scan.plan.lefttree = TranslateDXLBitmapAccessPath(
bitmap_access_path_dxlnode, output_context, md_rel, table_descr,
&base_table_context, ctxt_translation_prev_siblings, bitmap_tbl_scan);
- SetParamIds(plan);
if (is_parallel)
{
@@ -8606,7 +8543,6 @@ CTranslatorDXLToPlStmt::TranslateDXLBitmapIndexProbe(
* As of 8.4, the indexstrategy and indexsubtype fields are no longer
* available or needed in IndexScan. Ignore them.
*/
- SetParamIds(plan);
return plan;
}
diff --git a/src/backend/optimizer/plan/orca.c b/src/backend/optimizer/plan/orca.c
index a083fcda6c7..4b77babb188 100644
--- a/src/backend/optimizer/plan/orca.c
+++ b/src/backend/optimizer/plan/orca.c
@@ -45,6 +45,8 @@
#include "utils/syscache.h"
#include "catalog/pg_proc.h"
#include "catalog/pg_namespace.h"
+#include "cdb/cdbllize.h"
+#include "optimizer/walkers.h"
/* GPORCA entry point */
extern PlannedStmt * GPOPTOptimizedPlan(Query *parse, bool *had_unexpected_failure, OptimizerOptions *opts);
@@ -53,6 +55,7 @@ static Plan *remove_redundant_results(PlannerInfo *root, Plan *plan);
static Node *remove_redundant_results_mutator(Node *node, void *);
static bool can_replace_tlist(Plan *plan);
static Node *push_down_expr_mutator(Node *node, List *child_tlist);
+static bool set_plan_param_bitmaps_walker(Node *node, void *context);
/*
* Logging of optimization outcome
@@ -352,6 +355,28 @@ optimize_query(Query *parse, int cursorOptions, ParamListInfo boundParams, Optim
result->planTree = remove_redundant_results(root, result->planTree);
+ /*
+ * Compute the extParam/allParam bitmapsets of every Plan node, for the
+ * final shape of the plan.
+ *
+ * Rescan correctness depends on these: the executor propagates chgParam
+ * to a child only if the child's allParam contains the changed param
+ * (see UpdateChangedParamSet()), and e.g. a Material node relies on that
+ * to discard its tuplestore when a param of an enclosing SubPlan
+ * changes; a stale bitmap silently yields wrong results. Plans from the
+ * Postgres planner get these bits from SS_finalize_plan(); for ORCA
+ * plans we compute them here in one authoritative pass, instead of
+ * relying on every translator function to fill them in piecemeal.
+ */
+ {
+ plan_tree_base_prefix ctx;
+
+ ctx.node = (Node *) root;
+ foreach(lp, glob->subplans)
+ set_plan_param_bitmaps_walker((Node *) lfirst(lp), &ctx);
+ set_plan_param_bitmaps_walker((Node *) result->planTree, &ctx);
+ }
+
/*
* To save on memory, and on the network bandwidth when the plan is
* dispatched to QEs, strip all subquery RTEs of the original Query
@@ -412,6 +437,52 @@ optimize_query(Query *parse, int cursorOptions, ParamListInfo boundParams, Optim
return result;
}
+/*
+ * Set the extParam/allParam bitmapsets of every Plan node to the set of
+ * PARAM_EXEC params referenced in the node's subtree.
+ *
+ * SubPlan bodies are not entered here (extract_nodes_plan() collects the
+ * params of a SubPlan's testexpr and args, but does not descend into the
+ * body); the caller walks each entry of glob->subplans separately instead,
+ * so every plan node in the tree gets its bitmaps set exactly once.
+ */
+static bool
+set_plan_param_bitmaps_walker(Node *node, void *context)
+{
+ if (node == NULL)
+ return false;
+
+ if (is_plan_node(node))
+ {
+ Plan *plan = (Plan *) node;
+ List *params;
+ Bitmapset *bms = NULL;
+ ListCell *lc;
+
+ params = extract_nodes_plan(plan, T_Param,
+ true /* descendIntoSubqueries */);
+ foreach(lc, params)
+ {
+ Param *param = (Param *) lfirst(lc);
+
+ /*
+ * extParam/allParam must contain only PARAM_EXEC paramids (see
+ * plannodes.h); PARAM_EXTERN params live in a separate numbering
+ * space and never change during execution.
+ */
+ if (param->paramkind == PARAM_EXEC)
+ bms = bms_add_member(bms, param->paramid);
+ }
+ list_free(params);
+
+ plan->extParam = bms;
+ plan->allParam = bms;
+ }
+
+ return plan_tree_walker(node, set_plan_param_bitmaps_walker, context,
+ false /* recurse_into_subplans */);
+}
+
/*
* ORCA tends to generate gratuitous Result nodes for various reasons. We
* try to clean it up here, as much as we can, by eliminating the Results
diff --git a/src/include/gpopt/translate/CTranslatorDXLToPlStmt.h b/src/include/gpopt/translate/CTranslatorDXLToPlStmt.h
index 1b03df79961..fa4fb3f12fa 100644
--- a/src/include/gpopt/translate/CTranslatorDXLToPlStmt.h
+++ b/src/include/gpopt/translate/CTranslatorDXLToPlStmt.h
@@ -190,9 +190,6 @@ public:
static JoinType GetGPDBJoinTypeFromDXLJoinType(EdxlJoinType join_type);
private:
- // Set the bitmapset of a plan to the list of param_ids defined by the plan
- static void SetParamIds(Plan *);
-
static List *TranslatePartOids(IMdIdArray *parts, INT lockmode);
static List *TranslateJoinPruneParamids(
diff --git a/src/test/regress/expected/orca_material_rescan.out b/src/test/regress/expected/orca_material_rescan.out
new file mode 100644
index 00000000000..96e1ac3f5a3
--- /dev/null
+++ b/src/test/regress/expected/orca_material_rescan.out
@@ -0,0 +1,101 @@
+--
+-- Test rescan of Materialize in ORCA-translated correlated SubPlans.
+--
+-- A Materialize sitting above a subtree that consumes a param of an
+-- enclosing SubPlan (here: a ProjectSet computing UNNEST over an outer
+-- column) must discard its tuplestore and rebuild whenever the param
+-- changes. The executor propagates chgParam to a child only if the
+-- child's allParam bitmapset contains the changed param, so this hangs
+-- on the ORCA translator producing correct param bitmaps for every
+-- node, and on the Materialize not shielding a Motion-free child from
+-- rescans. When either is broken, the Materialize replays the first
+-- outer row's result for every subsequent row. GPDB 7.5.x shipped
+-- exactly this class of wrong-results regression.
+--
+-- The tables deliberately have no stats: with default cardinalities
+-- ORCA puts the ProjectSet on the nestloop inner side underneath a
+-- Materialize, which is the vulnerable plan shape. Keep them
+-- unanalyzed.
+--
+CREATE SCHEMA orca_material_rescan;
+SET search_path TO orca_material_rescan;
+CREATE TABLE ae_event (
+ seq_nvl numeric(10) NOT NULL,
+ ae_rule_id_ctn varchar(200) NULL,
+ ae_occr_dtm timestamp NULL
+) USING ao_row DISTRIBUTED RANDOMLY;
+CREATE TABLE ae_rule (
+ ae_rule_id varchar(50) NOT NULL,
+ ae_rule_nm varchar(200) NULL
+) USING ao_row DISTRIBUTED RANDOMLY;
+INSERT INTO ae_rule VALUES
+ ('R_01', 'Overheating'),
+ ('R_02', NULL),
+ ('R_03', 'Calibration Error'),
+ ('R_04', 'Sensor Offline');
+-- Edge cases: rule id lists with a duplicate, an id with no match in
+-- ae_rule, and empty lists. Every row must get its own aggregate, not
+-- a replay of the first row's.
+INSERT INTO ae_event VALUES
+ (1, 'R_01,R_02', '2026-06-10 08:59:49'),
+ (2, 'R_03', '2026-06-10 08:59:50'),
+ (3, '', '2026-06-10 08:59:51'),
+ (4, 'R_04,R_99', '2026-06-10 08:59:52'),
+ (5, 'R_01,R_01', '2026-06-10 08:59:53'),
+ (6, '', '2026-06-10 08:59:54');
+SELECT $query$
+SELECT seq_nvl, ae_rule_id_ctn
+ , ( SELECT string_agg(rul_nm, ',' ORDER BY ae_rule_id)
+ FROM ( SELECT coalesce(ae_rule_nm, ae_rule_id) AS rul_nm, ae_rule_id
+ FROM ae_rule t1
+ JOIN ( SELECT unnest(string_to_array(sh.ae_rule_id_ctn, ',')) ) t2(column2)
+ ON t1.ae_rule_id = t2.column2 ) t
+ ) AS rule_ctrl
+FROM ae_event sh
+WHERE sh.ae_occr_dtm >= '2026-06-10 06:00:00'
+ AND sh.ae_occr_dtm < '2026-06-11 06:00:00'
+ORDER BY seq_nvl
+$query$ AS qry \gset
+SET optimizer TO on;
+-- The shape under test: the SubPlan's nestloop inner side must be a
+-- Materialize over a Motion-free ProjectSet consuming the outer param.
+EXPLAIN (COSTS OFF)
+:qry ;
+ QUERY PLAN
+------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+ Sort
+ Sort Key: sh.seq_nvl
+ -> Result
+ -> Gather Motion 3:1 (slice1; segments: 3)
+ -> Seq Scan on ae_event sh
+ Filter: ((ae_occr_dtm >= 'Wed Jun 10 06:00:00 2026'::timestamp without time zone) AND (ae_occr_dtm < 'Thu Jun 11 06:00:00 2026'::timestamp without time zone))
+ SubPlan 1
+ -> Aggregate
+ -> Nested Loop
+ Join Filter: ((t1.ae_rule_id)::text = (unnest(string_to_array((sh.ae_rule_id_ctn)::text, ','::text))))
+ -> Materialize
+ -> Gather Motion 3:1 (slice2; segments: 3)
+ -> Seq Scan on ae_rule t1
+ -> Materialize
+ -> ProjectSet
+ -> Result
+ Optimizer: GPORCA
+(17 rows)
+
+:qry ;
+ seq_nvl | ae_rule_id_ctn | rule_ctrl
+---------+----------------+-------------------------
+ 1 | R_01,R_02 | Overheating,R_02
+ 2 | R_03 | Calibration Error
+ 3 | |
+ 4 | R_04,R_99 | Sensor Offline
+ 5 | R_01,R_01 | Overheating,Overheating
+ 6 | |
+(6 rows)
+
+RESET optimizer;
+RESET search_path;
+DROP SCHEMA orca_material_rescan CASCADE;
+NOTICE: drop cascades to 2 other objects
+DETAIL: drop cascades to table orca_material_rescan.ae_event
+drop cascades to table orca_material_rescan.ae_rule
diff --git a/src/test/regress/greenplum_schedule b/src/test/regress/greenplum_schedule
index 3dfb6193e7e..f3457a502a8 100755
--- a/src/test/regress/greenplum_schedule
+++ b/src/test/regress/greenplum_schedule
@@ -64,7 +64,7 @@ test: leastsquares opr_sanity_gp decode_expr bitmapscan bitmapscan_ao case_gp li
# below test(s) inject faults so each of them need to be in a separate group
test: gpcopy
-test: orca_static_pruning orca_groupingsets_fallbacks
+test: orca_static_pruning orca_groupingsets_fallbacks orca_material_rescan
test: filter gpctas gpdist gpdist_opclasses gpdist_legacy_opclasses matrix sublink table_functions olap_setup complex opclass_ddl information_schema guc_env_var gp_explain distributed_transactions explain_format olap_plans gp_copy_dtx
# below test(s) inject faults so each of them need to be in a separate group
test: guc_gp
diff --git a/src/test/regress/sql/orca_material_rescan.sql b/src/test/regress/sql/orca_material_rescan.sql
new file mode 100644
index 00000000000..bd11d5a3016
--- /dev/null
+++ b/src/test/regress/sql/orca_material_rescan.sql
@@ -0,0 +1,76 @@
+--
+-- Test rescan of Materialize in ORCA-translated correlated SubPlans.
+--
+-- A Materialize sitting above a subtree that consumes a param of an
+-- enclosing SubPlan (here: a ProjectSet computing UNNEST over an outer
+-- column) must discard its tuplestore and rebuild whenever the param
+-- changes. The executor propagates chgParam to a child only if the
+-- child's allParam bitmapset contains the changed param, so this hangs
+-- on the ORCA translator producing correct param bitmaps for every
+-- node, and on the Materialize not shielding a Motion-free child from
+-- rescans. When either is broken, the Materialize replays the first
+-- outer row's result for every subsequent row. GPDB 7.5.x shipped
+-- exactly this class of wrong-results regression.
+--
+-- The tables deliberately have no stats: with default cardinalities
+-- ORCA puts the ProjectSet on the nestloop inner side underneath a
+-- Materialize, which is the vulnerable plan shape. Keep them
+-- unanalyzed.
+--
+CREATE SCHEMA orca_material_rescan;
+SET search_path TO orca_material_rescan;
+
+CREATE TABLE ae_event (
+ seq_nvl numeric(10) NOT NULL,
+ ae_rule_id_ctn varchar(200) NULL,
+ ae_occr_dtm timestamp NULL
+) USING ao_row DISTRIBUTED RANDOMLY;
+
+CREATE TABLE ae_rule (
+ ae_rule_id varchar(50) NOT NULL,
+ ae_rule_nm varchar(200) NULL
+) USING ao_row DISTRIBUTED RANDOMLY;
+
+INSERT INTO ae_rule VALUES
+ ('R_01', 'Overheating'),
+ ('R_02', NULL),
+ ('R_03', 'Calibration Error'),
+ ('R_04', 'Sensor Offline');
+
+-- Edge cases: rule id lists with a duplicate, an id with no match in
+-- ae_rule, and empty lists. Every row must get its own aggregate, not
+-- a replay of the first row's.
+INSERT INTO ae_event VALUES
+ (1, 'R_01,R_02', '2026-06-10 08:59:49'),
+ (2, 'R_03', '2026-06-10 08:59:50'),
+ (3, '', '2026-06-10 08:59:51'),
+ (4, 'R_04,R_99', '2026-06-10 08:59:52'),
+ (5, 'R_01,R_01', '2026-06-10 08:59:53'),
+ (6, '', '2026-06-10 08:59:54');
+
+SELECT $query$
+SELECT seq_nvl, ae_rule_id_ctn
+ , ( SELECT string_agg(rul_nm, ',' ORDER BY ae_rule_id)
+ FROM ( SELECT coalesce(ae_rule_nm, ae_rule_id) AS rul_nm, ae_rule_id
+ FROM ae_rule t1
+ JOIN ( SELECT unnest(string_to_array(sh.ae_rule_id_ctn, ',')) ) t2(column2)
+ ON t1.ae_rule_id = t2.column2 ) t
+ ) AS rule_ctrl
+FROM ae_event sh
+WHERE sh.ae_occr_dtm >= '2026-06-10 06:00:00'
+ AND sh.ae_occr_dtm < '2026-06-11 06:00:00'
+ORDER BY seq_nvl
+$query$ AS qry \gset
+
+SET optimizer TO on;
+
+-- The shape under test: the SubPlan's nestloop inner side must be a
+-- Materialize over a Motion-free ProjectSet consuming the outer param.
+EXPLAIN (COSTS OFF)
+:qry ;
+
+:qry ;
+
+RESET optimizer;
+RESET search_path;
+DROP SCHEMA orca_material_rescan CASCADE;
--
2.54.0 (Apple Git-157)
|
Deneby67
added a commit
to Deneby67/cloudberry
that referenced
this pull request
Oct 20, 2026
Replace translator-local SetParamIds calls with a final-plan walk in optimize_query(), after redundant Results have been removed. Visit the main tree and every subplan, and include only PARAM_EXEC dependencies. Shield Material children from rescans only when their subtree contains a Motion. Free the temporary Motion list after checking it. Retain the ProjectSet regressions and cover a no-SRF Result with a one-time filter above a parameterized Limit/Sort/ProjectSet. The original PR returns 2,2,2,2 instead of 2,5,NULL,5 for changing inputs there. Add Material rescan coverage and repeated generic prepared executions. Adapted from the downstream patch shared in: apache#2149 (comment) Co-authored-by: Zhang Wenchao <zhangwenchao@apache.org> Assisted-by: Codex
Deneby67
changed the title
Compute parameter dependency bitmaps only after ProjectSet target lists and child links are complete. Otherwise correlated SRFs can lose rescan notifications and a Sort can return rows from a previous invocation. Recompute every generated ProjectSet and the optional top Result, so nested SRFs inherit parameters referenced below an OUTER_VAR projection. Add regression coverage for changing arrays, NULL/empty inputs, duplicates, and nested SRFs on colocated outer rows.
Replace translator-local SetParamIds calls with a final-plan walk in optimize_query(), after redundant Results have been removed. Visit the main tree and every subplan, and include only PARAM_EXEC dependencies. Shield Material children from rescans only when their subtree contains a Motion. Free the temporary Motion list after checking it. Retain the ProjectSet regressions and cover a no-SRF Result with a one-time filter above a parameterized Limit/Sort/ProjectSet. The original PR returns 2,2,2,2 instead of 2,5,NULL,5 for changing inputs there. Add Material rescan coverage and repeated generic prepared executions. Adapted from the downstream patch shared in: apache#2149 (comment) Co-authored-by: Zhang Wenchao <zhangwenchao@apache.org> Assisted-by: Codex
yjhjstz
force-pushed
the
fix-projectset-param-dependencies
branch
from
October 20, 2026 16:44
3724753 to
bce3c4b
Compare
TranslateDXLParallelAppend still called SetParamIds after parameter dependency computation moved to the final ORCA plan walk. Remove the orphaned call so the current main-based PR builds again. Assisted-by: Codex
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
4 participants
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.
ORCA plans can reuse results from an earlier outer row when parameter dependency bitmaps are computed before target lists or child links are complete. The original ProjectSet case is one example. A Result with a one-time filter above a correlated Limit/Sort/ProjectSet also returns
2,2,2,2for inputs whose expected aggregates are2,5,NULL,5.This updates the fix to the approach proposed in the review discussion:
SetParamIds()calls and compute dependencies afterremove_redundant_results(), over the final main tree and every subplan.PARAM_EXECinextParam/allParam; clientPARAM_EXTERNIDs use a separate numbering space.cdb_shield_child_from_rescansonly when its child subtree contains a Motion.The downstream patch was adapted to Apache main. The original array and nested-SRF regression cases are retained. Additional cases cover the no-SRF Result path, Material over a Motion-free correlated ProjectSet, and reuse of a generic prepared plan with different client parameters.
Validation
Completed a full
make -j12build andmake installfor the current PR tree based on main5d03c2c71e0, on Ubuntu 22.04 with--enable-debug --enable-cassert. Tested it on an isolated cluster with three primary segments.All eight selected regression tests passed with ORCA enabled and full plan comparison, without
--ignore-plans:Also checked:
2,2,2,2; the updated version returns2,5,NULL,5.debug_print_plan: an external-parameter-only scan and its ancestors have empty parameter bitmaps; Material shielding is true above Motion and false above the Motion-free ProjectSet.The full regression schedules have not been run.