From 89729a49d67e1e65875724759ad98c76d782a1ab Mon Sep 17 00:00:00 2001 From: jian he Date: Fri, 3 Jul 2026 11:27:32 +0800 Subject: [PATCH v50 2/2] 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 caa27ffe8e..1826475084 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); @@ -247,9 +257,9 @@ static void update_reduced_frame(WindowObject winobj, int64 pos); /* Forward declarations - NFA row evaluation */ static bool nfa_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 @@ -4013,20 +4023,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: * @@ -4038,23 +4036,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)); @@ -4142,8 +4134,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; @@ -4156,7 +4148,6 @@ static void eval_define_offsets(WindowAggState *winstate, List *defineClause) { EvalDefineOffsetsContext ctx; - NavTraversal trav; /* Defaults: no backward, no FIRST navigation */ winstate->navMaxOffset = 0; @@ -4172,12 +4163,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); } /* @@ -4202,6 +4190,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 94d9a45bdc..0f83db51e3 100644 --- a/src/backend/optimizer/plan/createplan.c +++ b/src/backend/optimizer/plan/createplan.c @@ -2486,9 +2486,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, @@ -2499,10 +2497,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 @@ -2533,6 +2529,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. @@ -2551,18 +2558,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 fac3d7bcf3..94c5b44be6 100644 --- a/src/backend/optimizer/plan/rpr.c +++ b/src/backend/optimizer/plan/rpr.c @@ -1945,36 +1945,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); -} -- 2.34.1