From e7e4e1b164e28004e5a6ae2f8d9cd1d2100c4271 Mon Sep 17 00:00:00 2001
From: Henson Choi <assam258@gmail.com>
Date: Wed, 17 Jun 2026 14:05:25 +0900
Subject: [PATCH v50 16/29] Further tidy up row pattern recognition plumbing

More behavior-neutral cleanups to the row pattern recognition code,
continuing the previous tidy-up and with no change to planner or
executor output:

- Drop the now-unused WindowClause argument of transformDefineClause()
  and flatten the redundant nested block in its DEFINE-variable loop.

- Replace manual ListCell iteration and index bookkeeping with
  foreach_node()/foreach_current_index() in
  validate_rpr_define_volatility() and nfa_evaluate_row(), and drop the
  redundant end-of-list break tests in nfa_evaluate_row() and
  nfa_reevaluate_dependent_vars() now that the loops walk
  defineClauseExprs directly.

- Test winstate->defineVariableList against NIL instead of
  list_length() > 0 in ExecInitWindowAgg().

- Remove the now-unused makefuncs.h include from plan/rpr.c.

- Fix the stale struct name in the RPCommonSyntax header comment.
---
 src/backend/executor/execRPR.c       |  3 -
 src/backend/executor/nodeWindowAgg.c |  9 +--
 src/backend/optimizer/plan/rpr.c     |  7 +--
 src/backend/parser/parse_rpr.c       | 84 +++++++++++++---------------
 src/include/nodes/parsenodes.h       |  2 +-
 5 files changed, 44 insertions(+), 61 deletions(-)

diff --git a/src/backend/executor/execRPR.c b/src/backend/executor/execRPR.c
index def0b8423b3..099b81aeb81 100644
--- a/src/backend/executor/execRPR.c
+++ b/src/backend/executor/execRPR.c
@@ -1618,9 +1618,6 @@ nfa_reevaluate_dependent_vars(WindowAggState *winstate, RPRNFAContext *ctx,
 			result = ExecEvalExpr(exprState, econtext, &isnull);
 			winstate->nfaVarMatched[varIdx] = (!isnull && DatumGetBool(result));
 		}
-
-		if (varIdx + 1 >= list_length(winstate->defineVariableList))
-			break;
 	}
 
 	/* Restore original match_start, currentpos, and invalidate cache */
diff --git a/src/backend/executor/nodeWindowAgg.c b/src/backend/executor/nodeWindowAgg.c
index 5d4832d1db9..819ad814bf5 100644
--- a/src/backend/executor/nodeWindowAgg.c
+++ b/src/backend/executor/nodeWindowAgg.c
@@ -3103,7 +3103,7 @@ ExecInitWindowAgg(WindowAgg *node, EState *estate, int eflags)
 	 * ordering (DEFINE order first), varId == defineIdx for all defined
 	 * variables, so no mapping is needed.
 	 */
-	if (list_length(winstate->defineVariableList) > 0)
+	if (winstate->defineVariableList != NIL)
 		winstate->nfaVarMatched = palloc0(sizeof(bool) *
 										  list_length(winstate->defineVariableList));
 	else
@@ -4642,8 +4642,6 @@ nfa_evaluate_row(WindowObject winobj, int64 pos, bool *varMatched)
 {
 	WindowAggState *winstate = winobj->winstate;
 	ExprContext *econtext = winstate->rprContext;
-	int			numDefineVars = list_length(winstate->defineVariableList);
-	int			varIdx = 0;
 	TupleTableSlot *slot;
 	int64		saved_pos;
 
@@ -4670,6 +4668,7 @@ nfa_evaluate_row(WindowObject winobj, int64 pos, bool *varMatched)
 
 	foreach_ptr(ExprState, exprState, winstate->defineClauseExprs)
 	{
+		int			varIdx = foreach_current_index(exprState);
 		Datum		result;
 		bool		isnull;
 
@@ -4677,10 +4676,6 @@ nfa_evaluate_row(WindowObject winobj, int64 pos, bool *varMatched)
 		result = ExecEvalExpr(exprState, econtext, &isnull);
 
 		varMatched[varIdx] = (!isnull && DatumGetBool(result));
-
-		varIdx++;
-		if (varIdx >= numDefineVars)
-			break;
 	}
 
 	winstate->currentpos = saved_pos;
diff --git a/src/backend/optimizer/plan/rpr.c b/src/backend/optimizer/plan/rpr.c
index 48b76a842ed..597a966c7b1 100644
--- a/src/backend/optimizer/plan/rpr.c
+++ b/src/backend/optimizer/plan/rpr.c
@@ -40,7 +40,6 @@
 #include "catalog/pg_proc.h"
 #include "mb/pg_wchar.h"
 #include "miscadmin.h"
-#include "nodes/makefuncs.h"
 #include "nodes/nodeFuncs.h"
 #include "optimizer/rpr.h"
 #include "tcop/tcopprot.h"
@@ -1887,12 +1886,8 @@ reject_volatile_in_define_walker(Node *node, void *context)
 void
 validate_rpr_define_volatility(List *defineClause)
 {
-	ListCell   *lc;
-
-	foreach(lc, defineClause)
+	foreach_node(TargetEntry, te, defineClause)
 	{
-		TargetEntry *te = lfirst_node(TargetEntry, lc);
-
 		(void) reject_volatile_in_define_walker((Node *) te->expr, NULL);
 	}
 }
diff --git a/src/backend/parser/parse_rpr.c b/src/backend/parser/parse_rpr.c
index 8ed01bb8f28..3e6b2e579a3 100644
--- a/src/backend/parser/parse_rpr.c
+++ b/src/backend/parser/parse_rpr.c
@@ -54,8 +54,8 @@ typedef struct
 /* Forward declarations */
 static void validateRPRPatternVarCount(ParseState *pstate, RPRPatternNode *node,
 									   List *rpDefs, List **varNames);
-static List *transformDefineClause(ParseState *pstate, WindowClause *wc,
-								   WindowDef *windef, List **targetlist);
+static List *transformDefineClause(ParseState *pstate, WindowDef *windef,
+								   List **targetlist);
 static bool define_walker(Node *node, void *context);
 
 /*
@@ -178,7 +178,7 @@ transformRPR(ParseState *pstate, WindowClause *wc, WindowDef *windef,
 	wc->initial = windef->rpCommonSyntax->initial;
 
 	/* Transform DEFINE clause into list of TargetEntry's */
-	wc->defineClause = transformDefineClause(pstate, wc, windef, targetlist);
+	wc->defineClause = transformDefineClause(pstate, windef, targetlist);
 
 	/* Store PATTERN AST for deparsing */
 	wc->rpPattern = windef->rpCommonSyntax->rpPattern;
@@ -313,7 +313,7 @@ validateRPRPatternVarCount(ParseState *pstate, RPRPatternNode *node,
  * parse_expr.c via the p_rpr_pattern_vars check.
  */
 static List *
-transformDefineClause(ParseState *pstate, WindowClause *wc, WindowDef *windef,
+transformDefineClause(ParseState *pstate, WindowDef *windef,
 					  List **targetlist)
 {
 	List	   *restargets;
@@ -345,6 +345,8 @@ transformDefineClause(ParseState *pstate, WindowClause *wc, WindowDef *windef,
 	foreach_node(ResTarget, restarget, windef->rpCommonSyntax->rpDefs)
 	{
 		TargetEntry *teDefine;
+		Node	   *expr;
+		List	   *vars;
 
 		name = restarget->name;
 
@@ -374,54 +376,48 @@ transformDefineClause(ParseState *pstate, WindowClause *wc, WindowDef *windef,
 		 * the individual Var nodes it references are present in the
 		 * targetlist, so the planner can propagate the referenced columns.
 		 */
-		{
-			Node	   *expr;
-			List	   *vars;
+		expr = transformExpr(pstate, restarget->val,
+							 EXPR_KIND_RPR_DEFINE);
 
-			expr = transformExpr(pstate, restarget->val,
-								 EXPR_KIND_RPR_DEFINE);
+		/*
+		 * Pull out Var nodes from the transformed expression and ensure each
+		 * one is present in the targetlist.  This is needed so the planner
+		 * propagates the referenced columns through the plan tree, making
+		 * them available to the WindowAgg's DEFINE evaluation.
+		 */
+		vars = pull_var_clause(expr, 0);
+		foreach_node(Var, var, vars)
+		{
+			bool		found = false;
 
-			/*
-			 * Pull out Var nodes from the transformed expression and ensure
-			 * each one is present in the targetlist.  This is needed so the
-			 * planner propagates the referenced columns through the plan
-			 * tree, making them available to the WindowAgg's DEFINE
-			 * evaluation.
-			 */
-			vars = pull_var_clause(expr, 0);
-			foreach_node(Var, var, vars)
+			foreach_node(TargetEntry, tle, *targetlist)
 			{
-				bool		found = false;
-
-				foreach_node(TargetEntry, tle, *targetlist)
+				if (IsA(tle->expr, Var) &&
+					((Var *) tle->expr)->varno == var->varno &&
+					((Var *) tle->expr)->varattno == var->varattno)
 				{
-					if (IsA(tle->expr, Var) &&
-						((Var *) tle->expr)->varno == var->varno &&
-						((Var *) tle->expr)->varattno == var->varattno)
-					{
-						found = true;
-						break;
-					}
-				}
-				if (!found)
-				{
-					TargetEntry *newtle;
-
-					newtle = makeTargetEntry((Expr *) copyObject(var),
-											 list_length(*targetlist) + 1,
-											 NULL,
-											 true);
-					*targetlist = lappend(*targetlist, newtle);
+					found = true;
+					break;
 				}
 			}
-			list_free(vars);
+			if (!found)
+			{
+				TargetEntry *newtle;
 
-			/* Build the defineClause entry directly from the transformed expr */
-			teDefine = makeTargetEntry((Expr *) expr,
-									   list_length(defineClause) + 1,
-									   pstrdup(name),
-									   true);
+				newtle = makeTargetEntry((Expr *) copyObject(var),
+										 list_length(*targetlist) + 1,
+										 NULL,
+										 true);
+				*targetlist = lappend(*targetlist, newtle);
+			}
 		}
+		list_free(vars);
+
+		/* Build the defineClause entry directly from the transformed expr */
+		teDefine = makeTargetEntry((Expr *) expr,
+								   list_length(defineClause) + 1,
+								   pstrdup(name),
+								   true);
 
 		/* build transformed DEFINE clause (list of TargetEntry) */
 		defineClause = lappend(defineClause, teDefine);
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index e309dfbdb66..947e668020e 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -644,7 +644,7 @@ typedef struct RPRPatternNode
 } RPRPatternNode;
 
 /*
- * RowPatternCommonSyntax - raw representation of row pattern common syntax
+ * RPCommonSyntax - raw representation of row pattern common syntax
  */
 typedef struct RPCommonSyntax
 {
-- 
2.50.1 (Apple Git-155)

