v50-0001-refactor-DEFINE-volatile-expression-check.nocfbot
application/octet-stream
Filename: v50-0001-refactor-DEFINE-volatile-expression-check.nocfbot
Type: application/octet-stream
Part: 0
Message:
Re: Row pattern recognition
From a4451f72219115db77c5a23e7a8e3fbfedbfd1ac Mon Sep 17 00:00:00 2001
From: jian he <jian.universality@gmail.com>
Date: Thu, 2 Jul 2026 11:44:36 +0800
Subject: [PATCH v50 1/3] refactor DEFINE volatile expression check
Based on https://github.com/assam258-5892/postgres/commits/RPR
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.
```
CREATE FUNCTION prev(integer) RETURNS integer AS 'SELECT -999'
LANGUAGE sql VOLATILE;
```
SQL language function will b inlined to constant by preprocess_expression, so
this is not a VOLATILE function.
Discussion: https://postgr.es/m/CACJufxG57=ddtbN=5RZCzhxWDYXvocKmB7NtZy+DoqZuhxb_DA@mail.gmail.com
---
src/backend/optimizer/plan/planner.c | 23 +++---
src/backend/optimizer/plan/rpr.c | 72 -------------------
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_base.out | 8 +--
src/test/regress/expected/rpr_integration.out | 2 -
src/test/regress/sql/rpr_base.sql | 7 +-
8 files changed, 21 insertions(+), 100 deletions(-)
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index f01c746615..0f1d347c9c 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -1049,26 +1049,25 @@ 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);
wc->endOffset = preprocess_expression(root, wc->endOffset,
EXPRKIND_LIMIT);
+
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 fac3d7bcf3..1f7e9b4980 100644
--- a/src/backend/optimizer/plan/rpr.c
+++ b/src/backend/optimizer/plan/rpr.c
@@ -1776,78 +1776,6 @@ computeAbsorbability(RPRPattern *pattern)
pattern->isAbsorbable = hasAbsorbable;
}
-/*
- * 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)
-{
- foreach_node(TargetEntry, te, defineClause)
- {
- (void) reject_volatile_in_define_walker((Node *) te->expr, NULL);
- }
-}
-
/*
* buildRPRPattern
* Compile pattern parse tree to flat bytecode array.
diff --git a/src/backend/parser/parse_rpr.c b/src/backend/parser/parse_rpr.c
index ac29c26e20..8cf6d49e90 100644
--- a/src/backend/parser/parse_rpr.c
+++ b/src/backend/parser/parse_rpr.c
@@ -441,7 +441,7 @@ transformDefineClause(ParseState *pstate, WindowDef *windef,
* or nested navigation operations
*
* 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 1de7f86f86..b3841d55a7 100644
--- a/src/include/optimizer/rpr.h
+++ b/src/include/optimizer/rpr.h
@@ -74,7 +74,6 @@
#define RPRElemIsFin(e) ((e)->varId == RPR_VARID_FIN)
#define RPRElemCanSkip(e) ((e)->min == 0)
-extern void validate_rpr_define_volatility(List *defineClause);
extern RPRPattern *buildRPRPattern(WindowClause *wc, bool hasMatchStartDependent);
/*
diff --git a/src/test/regress/expected/rpr.out b/src/test/regress/expected/rpr.out
index 52be54039e..241937b44a 100644
--- a/src/test/regress/expected/rpr.out
+++ b/src/test/regress/expected/rpr.out
@@ -1218,8 +1218,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 (
@@ -1255,8 +1253,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;
@@ -1269,8 +1265,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_base.out b/src/test/regress/expected/rpr_base.out
index eb9f43ca2c..1ce63a40ca 100644
--- a/src/test/regress/expected/rpr_base.out
+++ b/src/test/regress/expected/rpr_base.out
@@ -1971,8 +1971,10 @@ SELECT id, val, count(*) OVER w AS cnt, last_value(id) OVER w AS last_id
-- A qualified call invokes the function, so its volatility still matters
-- VOLATILE: unqualified is nav; qualified is rejected as a volatile function
-CREATE FUNCTION prev(integer) RETURNS integer AS 'SELECT -999'
- LANGUAGE sql VOLATILE;
+CREATE FUNCTION prev(integer) RETURNS INT AS $$
+BEGIN
+ RETURN -999;
+END; $$ LANGUAGE plpgsql VOLATILE;
SELECT id, val, count(*) OVER w AS cnt, last_value(id) OVER w AS last_id
FROM nt
WINDOW w AS (PARTITION BY g ORDER BY id
@@ -1997,8 +1999,6 @@ SELECT id, val, count(*) OVER w AS cnt, last_value(id) OVER w AS last_id
DEFINE A AS rpr_navns.prev(val) = -999)
ORDER BY id;
ERROR: volatile functions are not allowed in DEFINE clause
-LINE 6: DEFINE A AS rpr_navns.prev(val) = -999)
- ^
DROP FUNCTION prev(integer);
-- IMMUTABLE: unqualified is nav; qualified is the escape hatch and succeeds
CREATE FUNCTION prev(integer) RETURNS integer AS 'SELECT -999'
diff --git a/src/test/regress/expected/rpr_integration.out b/src/test/regress/expected/rpr_integration.out
index 541ad35de4..1c90e00527 100644
--- a/src/test/regress/expected/rpr_integration.out
+++ b/src/test/regress/expected/rpr_integration.out
@@ -1401,8 +1401,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
-- ============================================================
diff --git a/src/test/regress/sql/rpr_base.sql b/src/test/regress/sql/rpr_base.sql
index 61a5bcf0b8..c9ce59a723 100644
--- a/src/test/regress/sql/rpr_base.sql
+++ b/src/test/regress/sql/rpr_base.sql
@@ -1420,8 +1420,11 @@ SELECT id, val, count(*) OVER w AS cnt, last_value(id) OVER w AS last_id
-- A qualified call invokes the function, so its volatility still matters
-- VOLATILE: unqualified is nav; qualified is rejected as a volatile function
-CREATE FUNCTION prev(integer) RETURNS integer AS 'SELECT -999'
- LANGUAGE sql VOLATILE;
+CREATE FUNCTION prev(integer) RETURNS INT AS $$
+BEGIN
+ RETURN -999;
+END; $$ LANGUAGE plpgsql VOLATILE;
+
SELECT id, val, count(*) OVER w AS cnt, last_value(id) OVER w AS last_id
FROM nt
WINDOW w AS (PARTITION BY g ORDER BY id
--
2.34.1