From e98a8f27025adb0a09f30573b39cc32573b4c0f4 Mon Sep 17 00:00:00 2001 From: jian he Date: Fri, 3 Jul 2026 11:27:32 +0800 Subject: [PATCH] Drop the shared nav_traversal_walker for RPR DEFINE Both visit_nav_exec() and visit_nav_plan() assert: Assert(nav->arg == NULL || !IsA(nav->arg, RPRNavExpr)); Assert(nav->offset_arg == NULL || !IsA(nav->offset_arg, RPRNavExpr)); Assert(nav->compound_offset_arg == NULL || !IsA(nav->compound_offset_arg, RPRNavExpr)); so an RPRNavExpr is never nested inside another RPRNavExpr, and the expressions that contain RPRNavExpr nodes are not complicated. For that, the generic nav_traversal_walker() -- a NavTraversal struct carrying a function pointer and a void* context, shared between the planner and the executor is kind of complicated. Replace it with static RPRNavExpr_walker() in each site. RPRNavExpr_walker will finds RPRNavExpr nodes, and acts on them the way it wants. Having a small duplicated static walker in the two files is fine here, nodeWindowAgg.c no longer need to include "optimizer/rpr.h". rename visit_nav_plan to compute_matchStartDependent, rename visit_nav_exec to compute_nav_offsets. --- src/backend/executor/nodeWindowAgg.c | 65 ++++++++++++------------- src/backend/optimizer/plan/createplan.c | 28 ++++++----- src/backend/optimizer/plan/rpr.c | 33 ------------- 3 files changed, 48 insertions(+), 78 deletions(-) diff --git a/src/backend/executor/nodeWindowAgg.c b/src/backend/executor/nodeWindowAgg.c index 39f6080409a..057d859ba8b 100644 --- a/src/backend/executor/nodeWindowAgg.c +++ b/src/backend/executor/nodeWindowAgg.c @@ -47,7 +47,6 @@ #include "nodes/plannodes.h" #include "optimizer/clauses.h" #include "optimizer/optimizer.h" -#include "optimizer/rpr.h" #include "parser/parse_agg.h" #include "parser/parse_coerce.h" #include "utils/acl.h" @@ -177,6 +176,17 @@ typedef struct WindowStatePerAggData bool restart; /* need to restart this agg in this cycle? */ } WindowStatePerAggData; +typedef struct +{ + WindowAggState *winstate; + int64 maxOffset; /* max backward-reach offset across all nav + * exprs */ + bool maxOverflow; /* true if backward-reach overflow detected */ + int64 minFirstOffset; /* min forward-from-match_start offset; may be + * negative (PREV_FIRST: inner - outer < 0) */ + bool hasFirst; /* any FIRST-based nav found */ +} EvalDefineOffsetsContext; + static void initialize_windowaggregate(WindowAggState *winstate, WindowStatePerFunc perfuncstate, WindowStatePerAgg peraggstate); @@ -248,9 +258,9 @@ static void update_reduced_frame(WindowObject winobj, int64 pos); /* Forward declarations - DEFINE row evaluation */ static bool rpr_evaluate_row(WindowObject winobj, int64 pos, bool *varMatched); - -/* Forward declarations - navigation offset evaluation */ static void eval_define_offsets(WindowAggState *winstate, List *defineClause); +static bool RPRNavExpr_walker(Node *node, EvalDefineOffsetsContext *ctx); +static void compute_nav_offsets(RPRNavExpr *nav, EvalDefineOffsetsContext *context); /* * Not null info bit array consists of 2-bit items @@ -4014,20 +4024,8 @@ eval_nav_offset(WindowAggState *winstate, Expr *offset_expr) return offset; } -typedef struct -{ - WindowAggState *winstate; - int64 maxOffset; /* max backward-reach offset across all nav - * exprs */ - bool maxOverflow; /* true if backward-reach overflow detected */ - int64 minFirstOffset; /* min forward-from-match_start offset; may be - * negative (PREV_FIRST: inner - outer < 0) */ - bool hasFirst; /* any FIRST-based nav found */ -} EvalDefineOffsetsContext; - /* - * visit_nav_exec - * nav_traversal_walker callback (NavVisitFn) for the executor side. + * compute_nav_offsets * At each RPRNavExpr, evaluates the nav's offset expression(s) at * runtime via eval_nav_offset and accumulates: * @@ -4039,23 +4037,17 @@ typedef struct * compound NEXT_FIRST (= inner + outer, clamped to PG_INT64_MAX on * overflow; always >= 0 so never updates minFirstOffset in practice) * - * Counterpart of visit_nav_plan but using runtime evaluation instead of - * Const folding; runs only for offsets the planner marked NEEDS_EVAL. - * Match-start dependency is not recomputed here -- the planner's bitmapset - * is reused via winstate->defineMatchStartDependent. */ static void -visit_nav_exec(NavTraversal *t, RPRNavExpr *nav) +compute_nav_offsets(RPRNavExpr *nav, EvalDefineOffsetsContext *context) { int64 inner; int64 outer; - EvalDefineOffsetsContext *context = (EvalDefineOffsetsContext *) t->data; - /* - * Parser guarantee (mirrors visit_nav_plan): nav's direct children are - * never RPRNavExpr -- compound nesting is flattened in place and any - * other nesting is rejected. Outer-kind dispatch is sufficient. + * Parser guarantee (mirrors compute_matchStartDependent): nav's direct + * children are never RPRNavExpr -- compound nesting is flattened in place + * and any other nesting is rejected. Outer-kind dispatch is sufficient. */ Assert(nav->arg == NULL || !IsA(nav->arg, RPRNavExpr)); Assert(nav->offset_arg == NULL || !IsA(nav->offset_arg, RPRNavExpr)); @@ -4143,8 +4135,8 @@ visit_nav_exec(NavTraversal *t, RPRNavExpr *nav) * Compute navigation offsets for tuplestore trim from the DEFINE clause, * at executor init. * - * Evaluates every nav offset expression (constant or not) via visit_nav_exec - * and stores the results in the WindowAggState: + * Evaluates every nav offset expression via compute_nav_offsets and stores the + * results in the WindowAggState: * - navMaxOffset: max backward (PREV/LAST) reach; set to -1 (the retain-all * sentinel) on int64 overflow so advance_nav_mark() disables trim. * - hasFirstNav / navFirstOffset: forward (FIRST) reach from match_start; @@ -4157,7 +4149,6 @@ static void eval_define_offsets(WindowAggState *winstate, List *defineClause) { EvalDefineOffsetsContext ctx; - NavTraversal trav; /* Defaults: no backward, no FIRST navigation */ winstate->navMaxOffset = 0; @@ -4173,12 +4164,9 @@ eval_define_offsets(WindowAggState *winstate, List *defineClause) ctx.minFirstOffset = PG_INT64_MAX; ctx.hasFirst = false; - trav.visit = visit_nav_exec; - trav.data = &ctx; - foreach_node(TargetEntry, te, defineClause) { - nav_traversal_walker((Node *) te->expr, &trav); + RPRNavExpr_walker((Node *) te->expr, &ctx); } /* @@ -4203,6 +4191,17 @@ eval_define_offsets(WindowAggState *winstate, List *defineClause) } } +static bool +RPRNavExpr_walker(Node *node, EvalDefineOffsetsContext *ctx) +{ + if (node == NULL) + return false; + if (IsA(node, RPRNavExpr)) + compute_nav_offsets(castNode(RPRNavExpr, node), ctx); + + return expression_tree_walker(node, RPRNavExpr_walker, ctx); +} + /* * rpr_is_defined * Return true if row pattern recognition is defined. diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c index 5036ee89267..da0e439adb9 100644 --- a/src/backend/optimizer/plan/createplan.c +++ b/src/backend/optimizer/plan/createplan.c @@ -2492,9 +2492,7 @@ typedef struct DefineMetadataContext } DefineMetadataContext; /* - * visit_nav_plan - * nav_traversal_walker callback (NavVisitFn) for the planner side. - * At each RPRNavExpr in a DEFINE expression, computes: + * compute_matchStartDependent * * per-variable match_start dependency for absorption suppression: outer nav * kinds that reach match_start (FIRST, LAST-with-offset, PREV_FIRST, @@ -2505,10 +2503,8 @@ typedef struct DefineMetadataContext * prevent FIRST/LAST inside a PREV/NEXT value subexpression. */ static void -visit_nav_plan(NavTraversal *t, RPRNavExpr *nav) +compute_matchStartDependent(RPRNavExpr *nav, DefineMetadataContext *context) { - DefineMetadataContext *context = (DefineMetadataContext *) t->data; - /* * Parser guarantee: by the time the planner sees a DEFINE expression, * compound nesting has been flattened into a single RPRNavExpr and any @@ -2539,6 +2535,17 @@ visit_nav_plan(NavTraversal *t, RPRNavExpr *nav) context->curVarIdx); } +static bool +RPRNavExpr_walker(Node *node, DefineMetadataContext *ctx) +{ + if (node == NULL) + return false; + if (IsA(node, RPRNavExpr)) + compute_matchStartDependent(castNode(RPRNavExpr, node), ctx); + + return expression_tree_walker(node, RPRNavExpr_walker, ctx); +} + /* * compute_define_metadata * Classify which DEFINE variables depend on the match start. @@ -2557,18 +2564,15 @@ static void compute_define_metadata(List *defineClause, Bitmapset **matchStartDependent) { DefineMetadataContext ctx; - NavTraversal trav; ctx.curVarIdx = 0; ctx.matchStartDependent = NULL; - trav.visit = visit_nav_plan; - trav.data = &ctx; - foreach_node(TargetEntry, te, defineClause) { - nav_traversal_walker((Node *) te->expr, &trav); - ctx.curVarIdx++; + ctx.curVarIdx = foreach_current_index(te); + + RPRNavExpr_walker((Node *) te->expr, &ctx); } *matchStartDependent = ctx.matchStartDependent; diff --git a/src/backend/optimizer/plan/rpr.c b/src/backend/optimizer/plan/rpr.c index 91f88b0ce88..fc85ebeec44 100644 --- a/src/backend/optimizer/plan/rpr.c +++ b/src/backend/optimizer/plan/rpr.c @@ -2002,36 +2002,3 @@ buildRPRPattern(WindowClause *wc, bool hasMatchStartDependent) return result; } - -/* - * nav_traversal_walker - * Shared expression-tree walker that locates RPRNavExpr nodes in a - * DEFINE expression and dispatches each one to a caller-supplied - * visitor. Used by: - * - planner (visit_nav_plan in createplan.c) to collect tuplestore - * trim offsets and per-variable match_start dependency - * - executor (visit_nav_exec in nodeWindowAgg.c) to evaluate - * non-constant nav offsets at WindowAggState init time - * - * The driver wraps a mode-specific context in a NavTraversal and passes - * it as ctx; the visitor casts t->data to its own context type. Children - * of an RPRNavExpr are not walked: the parser's nesting restrictions - * ensure offsets and dependencies are fully captured by the outer nav - * kind, so the visitor only needs to inspect the RPRNavExpr itself. - */ -bool -nav_traversal_walker(Node *node, void *ctx) -{ - if (node == NULL) - return false; - - if (IsA(node, RPRNavExpr)) - { - NavTraversal *t = (NavTraversal *) ctx; - - t->visit(t, (RPRNavExpr *) node); - return false; - } - - return expression_tree_walker(node, nav_traversal_walker, ctx); -}