v50-0002-Drop-the-shared-nav_traversal_walker-for-RPR-DEFINE.nocfbot
application/octet-stream
Filename: v50-0002-Drop-the-shared-nav_traversal_walker-for-RPR-DEFINE.nocfbot
Type: application/octet-stream
Part: 0
Message:
Re: Row pattern recognition
From 89729a49d67e1e65875724759ad98c76d782a1ab Mon Sep 17 00:00:00 2001
From: jian he <jian.universality@gmail.com>
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