From 4cf9108ce7796a3821873f8377c95e34799760f8 Mon Sep 17 00:00:00 2001 From: Henson Choi Date: Wed, 17 Jun 2026 14:05:25 +0900 Subject: [PATCH 07/13] Further tidy up row pattern recognition plumbing More behavior-neutral cleanups to the row pattern recognition code, continuing the previous tidy-up and with no change to planner or executor output: - Drop the now-unused WindowClause argument of transformDefineClause() and flatten the redundant nested block in its DEFINE-variable loop. - Replace manual ListCell iteration and index bookkeeping with foreach_node()/foreach_current_index() in validate_rpr_define_volatility() and nfa_evaluate_row(), and drop the redundant end-of-list break tests in nfa_evaluate_row() and nfa_reevaluate_dependent_vars() now that the loops walk defineClauseExprs directly. - Test winstate->defineVariableList against NIL instead of list_length() > 0 in ExecInitWindowAgg(). - Remove the now-unused makefuncs.h include from plan/rpr.c. - Fix the stale struct name in the RPCommonSyntax header comment. --- src/backend/executor/execRPR.c | 3 - src/backend/executor/nodeWindowAgg.c | 9 +-- src/backend/optimizer/plan/rpr.c | 7 +-- src/backend/parser/parse_rpr.c | 84 +++++++++++++--------------- src/include/nodes/parsenodes.h | 2 +- 5 files changed, 44 insertions(+), 61 deletions(-) diff --git a/src/backend/executor/execRPR.c b/src/backend/executor/execRPR.c index def0b8423b3..099b81aeb81 100644 --- a/src/backend/executor/execRPR.c +++ b/src/backend/executor/execRPR.c @@ -1618,9 +1618,6 @@ nfa_reevaluate_dependent_vars(WindowAggState *winstate, RPRNFAContext *ctx, result = ExecEvalExpr(exprState, econtext, &isnull); winstate->nfaVarMatched[varIdx] = (!isnull && DatumGetBool(result)); } - - if (varIdx + 1 >= list_length(winstate->defineVariableList)) - break; } /* Restore original match_start, currentpos, and invalidate cache */ diff --git a/src/backend/executor/nodeWindowAgg.c b/src/backend/executor/nodeWindowAgg.c index 5d4832d1db9..819ad814bf5 100644 --- a/src/backend/executor/nodeWindowAgg.c +++ b/src/backend/executor/nodeWindowAgg.c @@ -3103,7 +3103,7 @@ ExecInitWindowAgg(WindowAgg *node, EState *estate, int eflags) * ordering (DEFINE order first), varId == defineIdx for all defined * variables, so no mapping is needed. */ - if (list_length(winstate->defineVariableList) > 0) + if (winstate->defineVariableList != NIL) winstate->nfaVarMatched = palloc0(sizeof(bool) * list_length(winstate->defineVariableList)); else @@ -4642,8 +4642,6 @@ nfa_evaluate_row(WindowObject winobj, int64 pos, bool *varMatched) { WindowAggState *winstate = winobj->winstate; ExprContext *econtext = winstate->rprContext; - int numDefineVars = list_length(winstate->defineVariableList); - int varIdx = 0; TupleTableSlot *slot; int64 saved_pos; @@ -4670,6 +4668,7 @@ nfa_evaluate_row(WindowObject winobj, int64 pos, bool *varMatched) foreach_ptr(ExprState, exprState, winstate->defineClauseExprs) { + int varIdx = foreach_current_index(exprState); Datum result; bool isnull; @@ -4677,10 +4676,6 @@ nfa_evaluate_row(WindowObject winobj, int64 pos, bool *varMatched) result = ExecEvalExpr(exprState, econtext, &isnull); varMatched[varIdx] = (!isnull && DatumGetBool(result)); - - varIdx++; - if (varIdx >= numDefineVars) - break; } winstate->currentpos = saved_pos; diff --git a/src/backend/optimizer/plan/rpr.c b/src/backend/optimizer/plan/rpr.c index 48b76a842ed..597a966c7b1 100644 --- a/src/backend/optimizer/plan/rpr.c +++ b/src/backend/optimizer/plan/rpr.c @@ -40,7 +40,6 @@ #include "catalog/pg_proc.h" #include "mb/pg_wchar.h" #include "miscadmin.h" -#include "nodes/makefuncs.h" #include "nodes/nodeFuncs.h" #include "optimizer/rpr.h" #include "tcop/tcopprot.h" @@ -1887,12 +1886,8 @@ reject_volatile_in_define_walker(Node *node, void *context) void validate_rpr_define_volatility(List *defineClause) { - ListCell *lc; - - foreach(lc, defineClause) + foreach_node(TargetEntry, te, defineClause) { - TargetEntry *te = lfirst_node(TargetEntry, lc); - (void) reject_volatile_in_define_walker((Node *) te->expr, NULL); } } diff --git a/src/backend/parser/parse_rpr.c b/src/backend/parser/parse_rpr.c index 8ed01bb8f28..3e6b2e579a3 100644 --- a/src/backend/parser/parse_rpr.c +++ b/src/backend/parser/parse_rpr.c @@ -54,8 +54,8 @@ typedef struct /* Forward declarations */ static void validateRPRPatternVarCount(ParseState *pstate, RPRPatternNode *node, List *rpDefs, List **varNames); -static List *transformDefineClause(ParseState *pstate, WindowClause *wc, - WindowDef *windef, List **targetlist); +static List *transformDefineClause(ParseState *pstate, WindowDef *windef, + List **targetlist); static bool define_walker(Node *node, void *context); /* @@ -178,7 +178,7 @@ transformRPR(ParseState *pstate, WindowClause *wc, WindowDef *windef, wc->initial = windef->rpCommonSyntax->initial; /* Transform DEFINE clause into list of TargetEntry's */ - wc->defineClause = transformDefineClause(pstate, wc, windef, targetlist); + wc->defineClause = transformDefineClause(pstate, windef, targetlist); /* Store PATTERN AST for deparsing */ wc->rpPattern = windef->rpCommonSyntax->rpPattern; @@ -313,7 +313,7 @@ validateRPRPatternVarCount(ParseState *pstate, RPRPatternNode *node, * parse_expr.c via the p_rpr_pattern_vars check. */ static List * -transformDefineClause(ParseState *pstate, WindowClause *wc, WindowDef *windef, +transformDefineClause(ParseState *pstate, WindowDef *windef, List **targetlist) { List *restargets; @@ -345,6 +345,8 @@ transformDefineClause(ParseState *pstate, WindowClause *wc, WindowDef *windef, foreach_node(ResTarget, restarget, windef->rpCommonSyntax->rpDefs) { TargetEntry *teDefine; + Node *expr; + List *vars; name = restarget->name; @@ -374,54 +376,48 @@ 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); + /* + * 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; - /* - * 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) + foreach_node(TargetEntry, tle, *targetlist) { - 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) { - 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); + found = true; + break; } } - list_free(vars); + if (!found) + { + TargetEntry *newtle; - /* Build the defineClause entry directly from the transformed expr */ - teDefine = makeTargetEntry((Expr *) expr, - list_length(defineClause) + 1, - pstrdup(name), - true); + 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 transformed DEFINE clause (list of TargetEntry) */ defineClause = lappend(defineClause, teDefine); diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index e309dfbdb66..947e668020e 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -644,7 +644,7 @@ typedef struct RPRPatternNode } RPRPatternNode; /* - * RowPatternCommonSyntax - raw representation of row pattern common syntax + * RPCommonSyntax - raw representation of row pattern common syntax */ typedef struct RPCommonSyntax { -- 2.50.1 (Apple Git-155)