v47-0005-refactor-transformDefineClause.nocfbot

From b32fc75387f499f42d8ab7c5c2491f3cb3a3c90c Mon Sep 17 00:00:00 2001
From: jian he <jian.universality@gmail.com>
Date: Fri, 12 Jun 2026 18:33:37 +0800
Subject: [PATCH v47 5/6] refactor transformDefineClause
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

coerce_to_boolean() was deferred to a second pass over the completed
defineClause list, meaning pull_var_clause() ran on an expression that had not
yet been fully coerced/transformed. The more intuitively order should be:
transformExpr → coerce_to_boolean → pull_var_clause, so pull_var_clause always
sees the final expression form.
---
 src/backend/parser/parse_rpr.c         | 142 ++++++++++++-------------
 src/test/regress/expected/rpr_base.out |   2 +-
 2 files changed, 67 insertions(+), 77 deletions(-)

diff --git a/src/backend/parser/parse_rpr.c b/src/backend/parser/parse_rpr.c
index 8521c8ee9f..59624c7a46 100644
--- a/src/backend/parser/parse_rpr.c
+++ b/src/backend/parser/parse_rpr.c
@@ -284,9 +284,7 @@ static List *
 transformDefineClause(ParseState *pstate, WindowClause *wc, WindowDef *windef,
 					  List **targetlist)
 {
-	List	   *restargets;
 	List	   *defineClause = NIL;
-	char	   *name;
 	List	   *patternVarNames = NIL;
 
 	/*
@@ -326,32 +324,36 @@ transformDefineClause(ParseState *pstate, WindowClause *wc, WindowDef *windef,
 	pstate->p_rpr_pattern_vars = patternVarNames;
 
 	/*
-	 * Check for duplicate row pattern definition variables.  The standard
+	 * Check for duplicate row pattern definition variables.  The SQL standard
 	 * requires that no two row pattern definition variable names shall be
 	 * equivalent.
 	 */
-	restargets = NIL;
+	for (int outerpos = 0; outerpos < list_length(windef->rpCommonSyntax->rpDefs); outerpos++)
+	{
+		ResTarget  *restarget = list_nth_node(ResTarget,
+											  windef->rpCommonSyntax->rpDefs,
+											  outerpos);
+
+		for (int restpos = outerpos + 1; restpos < list_length(windef->rpCommonSyntax->rpDefs); restpos++)
+		{
+			ResTarget  *inner_res = list_nth_node(ResTarget,
+												  windef->rpCommonSyntax->rpDefs,
+												  restpos);
+
+			if (!strcmp(restarget->name, inner_res->name))
+				ereport(ERROR,
+						errcode(ERRCODE_SYNTAX_ERROR),
+						errmsg("DEFINE variable \"%s\" appears more than once",
+							   restarget->name),
+						parser_errposition(pstate, exprLocation((Node *) inner_res)));
+		}
+	}
+
 	foreach_node(ResTarget, restarget, windef->rpCommonSyntax->rpDefs)
 	{
 		TargetEntry *teDefine;
-
-		name = restarget->name;
-
-		foreach_node(ResTarget, r, restargets)
-		{
-			char	   *n;
-
-			n = r->name;
-
-			if (!strcmp(n, name))
-				ereport(ERROR,
-						errcode(ERRCODE_SYNTAX_ERROR),
-						errmsg("DEFINE variable \"%s\" appears more than once",
-							   name),
-						parser_errposition(pstate, exprLocation((Node *) r)));
-		}
-
-		restargets = lappend(restargets, restarget);
+		Node	   *expr;
+		List	   *vars;
 
 		/*
 		 * Transform the DEFINE expression.  We must NOT add the whole
@@ -363,68 +365,56 @@ 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);
+		expr = coerce_to_boolean(pstate, expr, "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;
-
-				foreach_node(TargetEntry, tle, *targetlist)
-				{
-					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);
-				}
-			}
-			list_free(vars);
-
-			/* Build the defineClause entry directly from the transformed expr */
-			teDefine = makeTargetEntry((Expr *) expr,
-									   list_length(defineClause) + 1,
-									   pstrdup(name),
-									   true);
-		}
+		/* Build the defineClause entry directly from the transformed expr */
+		teDefine = makeTargetEntry((Expr *) expr,
+								   list_length(defineClause) + 1,
+								   pstrdup(restarget->name),
+								   true);
 
 		/* build transformed DEFINE clause (list of TargetEntry) */
 		defineClause = lappend(defineClause, teDefine);
+
+		/*
+		 * 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;
+
+			foreach_node(TargetEntry, tle, *targetlist)
+			{
+				if (equal(tle->expr, var))
+				{
+					found = true;
+					break;
+				}
+			}
+
+			if (!found)
+			{
+				TargetEntry *newtle;
+
+				newtle = makeTargetEntry((Expr *) copyObject(var),
+										 list_length(*targetlist) + 1,
+										 NULL,
+										 true);
+				*targetlist = lappend(*targetlist, newtle);
+			}
+		}
+		list_free(vars);
 	}
-	list_free(restargets);
+
 	pstate->p_rpr_pattern_vars = NIL;
 
-	/*
-	 * Make sure that the row pattern definition search condition is a boolean
-	 * expression.
-	 */
-	foreach_ptr(TargetEntry, te, defineClause)
-		te->expr = (Expr *) coerce_to_boolean(pstate, (Node *) te->expr, "DEFINE");
-
 	/*
 	 * Validate DEFINE expressions: nested PREV/NEXT, column references,
 	 * compound flatten, volatile callees -- all in a single walk per
diff --git a/src/test/regress/expected/rpr_base.out b/src/test/regress/expected/rpr_base.out
index 41541898f5..cae5cab0d8 100644
--- a/src/test/regress/expected/rpr_base.out
+++ b/src/test/regress/expected/rpr_base.out
@@ -236,7 +236,7 @@ WINDOW w AS (
 );
 ERROR:  DEFINE variable "a" appears more than once
 LINE 7:     DEFINE A AS id > 0, A AS id < 10
-                   ^
+                                ^
 DROP TABLE rpr_dup;
 -- Boolean coercion
 CREATE TABLE rpr_bool (id INT, flag BOOLEAN);
-- 
2.34.1