From 2fd2878ce32b2bca441f4ff06d77ad0a0a03a9fc Mon Sep 17 00:00:00 2001
From: jian he <jian.universality@gmail.com>
Date: Fri, 12 Jun 2026 18:29:35 +0800
Subject: [PATCH v47 3/6] refactor volatile expression check

validate_rpr_define_volatility function, along with its helper functions is removed.
contain_volatile_functions() already covers both volatile FuncExpr callees and
NextValueExpr, so a separate walker is not needed. The trade-off is that we lose
the error cursor position, but that seems better than to maintaining extra code.

The check is also moved to after preprocess_expression(), following the same
pattern as contain_volatile_functions_after_planning.
---
 src/backend/optimizer/plan/planner.c          | 22 +++---
 src/backend/optimizer/plan/rpr.c              | 76 -------------------
 src/backend/parser/parse_rpr.c                |  2 +-
 src/include/optimizer/rpr.h                   |  1 -
 src/test/regress/expected/rpr.out             |  6 --
 src/test/regress/expected/rpr_integration.out |  2 -
 6 files changed, 11 insertions(+), 98 deletions(-)

diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index d20015cd86..f7f2869107 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -1049,18 +1049,6 @@ subquery_planner(PlannerGlobal *glob, Query *parse, char *plan_name,
 	{
 		WindowClause *wc = lfirst_node(WindowClause, l);
 
-		/*
-		 * Reject volatile functions (and sequence operations) in an RPR
-		 * DEFINE clause.  This is done here, not during parse analysis, to
-		 * follow the convention of not checking expression volatility while
-		 * parsing; debug_query_string still lets us report the offending
-		 * location.  Every window clause is visited, including ones not used
-		 * by any OVER, so the check does not depend on the window surviving
-		 * select_active_windows().
-		 */
-		if (wc->rpPattern && wc->defineClause)
-			validate_rpr_define_volatility(wc->defineClause);
-
 		/* partitionClause/orderClause are sort/group expressions */
 		wc->startOffset = preprocess_expression(root, wc->startOffset,
 												EXPRKIND_LIMIT);
@@ -1069,6 +1057,16 @@ subquery_planner(PlannerGlobal *glob, Query *parse, char *plan_name,
 		wc->defineClause = (List *) preprocess_expression(root,
 														  (Node *) wc->defineClause,
 														  EXPRKIND_TARGET);
+
+		/*
+		 * Reject volatile expressions in an RPR DEFINE clause.  This is done
+		 * here, not during parse analysis, to follow the convention of not
+		 * checking expression volatility while parsing.
+		 */
+		if (contain_volatile_functions((Node *) wc->defineClause))
+			ereport(ERROR,
+					errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+					errmsg("volatile functions are not allowed in DEFINE clause"));
 	}
 
 	parse->limitOffset = preprocess_expression(root, parse->limitOffset,
diff --git a/src/backend/optimizer/plan/rpr.c b/src/backend/optimizer/plan/rpr.c
index 175777a8ff..e77bf93eaf 100644
--- a/src/backend/optimizer/plan/rpr.c
+++ b/src/backend/optimizer/plan/rpr.c
@@ -1879,82 +1879,6 @@ collectPatternVariables(RPRPatternNode *pattern)
 	return varNames;
 }
 
-/*
- * rpr_volatile_func_checker
- *		check_functions_in_node callback: true if funcid is VOLATILE.
- */
-static bool
-rpr_volatile_func_checker(Oid funcid, void *context)
-{
-	return (func_volatile(funcid) == PROVOLATILE_VOLATILE);
-}
-
-/*
- * rpr_define_errposition
- *		Error cursor position for a DEFINE subexpression.
- *
- * The planner has no ParseState, but the original query text is available in
- * debug_query_string, so we can still point at the offending location exactly
- * as parser_errposition() would.
- */
-static int
-rpr_define_errposition(int location)
-{
-	if (location < 0 || debug_query_string == NULL)
-		return 0;
-	return errposition(pg_mbstrlen_with_len(debug_query_string, location) + 1);
-}
-
-/*
- * reject_volatile_in_define_walker
- *		Reject volatile callees and sequence operations anywhere in a DEFINE
- *		expression: they are non-deterministic across the multiple predicate
- *		evaluations that NFA backtracking and PREV/NEXT navigation may trigger
- *		for a single row.
- *
- * NextValueExpr is checked separately because it is not a function call and
- * so is not caught by check_functions_in_node().
- */
-static bool
-reject_volatile_in_define_walker(Node *node, void *context)
-{
-	if (node == NULL)
-		return false;
-	if (check_functions_in_node(node, rpr_volatile_func_checker, NULL))
-		ereport(ERROR,
-				errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-				errmsg("volatile functions are not allowed in DEFINE clause"),
-				rpr_define_errposition(exprLocation(node)));
-	if (IsA(node, NextValueExpr))
-		ereport(ERROR,
-				errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-				errmsg("sequence operations are not allowed in DEFINE clause"),
-				rpr_define_errposition(exprLocation(node)));
-	return expression_tree_walker(node, reject_volatile_in_define_walker, context);
-}
-
-/*
- * validate_rpr_define_volatility
- *		Reject volatile functions / sequence operations in a DEFINE clause.
- *
- * Called from the planner (subquery_planner) for every RPR WindowClause,
- * including windows not referenced by any OVER clause, so the check is applied
- * regardless of whether the window survives to execution -- matching the
- * coverage of the former parse-time check.
- */
-void
-validate_rpr_define_volatility(List *defineClause)
-{
-	ListCell   *lc;
-
-	foreach(lc, defineClause)
-	{
-		TargetEntry *te = lfirst_node(TargetEntry, lc);
-
-		(void) reject_volatile_in_define_walker((Node *) te->expr, NULL);
-	}
-}
-
 /*
  * buildDefineVariableList
  *		Build defineVariableList from defineClause.
diff --git a/src/backend/parser/parse_rpr.c b/src/backend/parser/parse_rpr.c
index a21aadae73..7c7458f933 100644
--- a/src/backend/parser/parse_rpr.c
+++ b/src/backend/parser/parse_rpr.c
@@ -473,7 +473,7 @@ transformDefineClause(ParseState *pstate, WindowClause *wc, WindowDef *windef,
  *     * offset_arg / compound_offset_arg must not contain column refs
  *
  * Volatile callees (and sequence operations) are rejected later in the
- * planner via validate_rpr_define_volatility(); see optimizer/plan/rpr.c.
+ * planner time in subquery_planner.
  *
  * The walker uses a phase tag to know which subtree it is in: DEFINE
  * body (top-level), inside a nav.arg, or inside a nav.offset_arg /
diff --git a/src/include/optimizer/rpr.h b/src/include/optimizer/rpr.h
index 802d2f1dd6..8e0bc7efc5 100644
--- a/src/include/optimizer/rpr.h
+++ b/src/include/optimizer/rpr.h
@@ -67,7 +67,6 @@
 #define RPRElemCanSkip(e)		((e)->min == 0)
 
 extern List *collectPatternVariables(RPRPatternNode *pattern);
-extern void validate_rpr_define_volatility(List *defineClause);
 extern void buildDefineVariableList(List *defineClause,
 									List **defineVariableList);
 extern RPRPattern *buildRPRPattern(RPRPatternNode *pattern, List *defineVariableList,
diff --git a/src/test/regress/expected/rpr.out b/src/test/regress/expected/rpr.out
index c02c9d75a9..a07104153f 100644
--- a/src/test/regress/expected/rpr.out
+++ b/src/test/regress/expected/rpr.out
@@ -1241,8 +1241,6 @@ WINDOW w AS (
     DEFINE A AS PREV(price, random()::int) > 0
 );
 ERROR:  volatile functions are not allowed in DEFINE clause
-LINE 7:     DEFINE A AS PREV(price, random()::int) > 0
-                                    ^
 -- Non-constant offset: subquery as offset
 SELECT price FROM stock
 WINDOW w AS (
@@ -1278,8 +1276,6 @@ WINDOW w AS (
     DEFINE A AS PREV(price + random() * 0) >= 0
 );
 ERROR:  volatile functions are not allowed in DEFINE clause
-LINE 8:     DEFINE A AS PREV(price + random() * 0) >= 0
-                                     ^
 -- nextval is volatile (per pg_proc), so it is rejected via the FuncExpr
 -- path with the "volatile functions" message
 CREATE SEQUENCE rpr_seq;
@@ -1292,8 +1288,6 @@ WINDOW w AS (
     DEFINE A AS price > nextval('rpr_seq')
 );
 ERROR:  volatile functions are not allowed in DEFINE clause
-LINE 7:     DEFINE A AS price > nextval('rpr_seq')
-                                ^
 DROP SEQUENCE rpr_seq;
 -- A volatile DEFINE is now rejected in the planner, not at parse time, so a
 -- view that hides one is created successfully and only errors when read.
diff --git a/src/test/regress/expected/rpr_integration.out b/src/test/regress/expected/rpr_integration.out
index 80a32cca9a..57913a8f69 100644
--- a/src/test/regress/expected/rpr_integration.out
+++ b/src/test/regress/expected/rpr_integration.out
@@ -1408,8 +1408,6 @@ WINDOW w AS (ORDER BY id
     DEFINE B AS val > PREV(val) AND random() >= 0.0)
 ORDER BY id;
 ERROR:  volatile functions are not allowed in DEFINE clause
-LINE 6:     DEFINE B AS val > PREV(val) AND random() >= 0.0)
-                                            ^
 -- ============================================================
 -- B10. RPR + Correlated subquery in WHERE
 -- ============================================================
-- 
2.34.1

