From 2fd2878ce32b2bca441f4ff06d77ad0a0a03a9fc Mon Sep 17 00:00:00 2001 From: jian he 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