v47-0003-refactor-volatile-expression-check.nocfbot
application/octet-stream
Filename: v47-0003-refactor-volatile-expression-check.nocfbot
Type: application/octet-stream
Part: 0
Message:
Re: Row pattern recognition
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