From b32fc75387f499f42d8ab7c5c2491f3cb3a3c90c Mon Sep 17 00:00:00 2001 From: jian he Date: Fri, 12 Jun 2026 18:33:37 +0800 Subject: [PATCH v47 5/6] refactor transformDefineClause MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit coerce_to_boolean() was deferred to a second pass over the completed defineClause list, meaning pull_var_clause() ran on an expression that had not yet been fully coerced/transformed. The more intuitively order should be: transformExpr → coerce_to_boolean → pull_var_clause, so pull_var_clause always sees the final expression form. --- src/backend/parser/parse_rpr.c | 142 ++++++++++++------------- src/test/regress/expected/rpr_base.out | 2 +- 2 files changed, 67 insertions(+), 77 deletions(-) diff --git a/src/backend/parser/parse_rpr.c b/src/backend/parser/parse_rpr.c index 8521c8ee9f..59624c7a46 100644 --- a/src/backend/parser/parse_rpr.c +++ b/src/backend/parser/parse_rpr.c @@ -284,9 +284,7 @@ static List * transformDefineClause(ParseState *pstate, WindowClause *wc, WindowDef *windef, List **targetlist) { - List *restargets; List *defineClause = NIL; - char *name; List *patternVarNames = NIL; /* @@ -326,32 +324,36 @@ transformDefineClause(ParseState *pstate, WindowClause *wc, WindowDef *windef, pstate->p_rpr_pattern_vars = patternVarNames; /* - * Check for duplicate row pattern definition variables. The standard + * Check for duplicate row pattern definition variables. The SQL standard * requires that no two row pattern definition variable names shall be * equivalent. */ - restargets = NIL; + for (int outerpos = 0; outerpos < list_length(windef->rpCommonSyntax->rpDefs); outerpos++) + { + ResTarget *restarget = list_nth_node(ResTarget, + windef->rpCommonSyntax->rpDefs, + outerpos); + + for (int restpos = outerpos + 1; restpos < list_length(windef->rpCommonSyntax->rpDefs); restpos++) + { + ResTarget *inner_res = list_nth_node(ResTarget, + windef->rpCommonSyntax->rpDefs, + restpos); + + if (!strcmp(restarget->name, inner_res->name)) + ereport(ERROR, + errcode(ERRCODE_SYNTAX_ERROR), + errmsg("DEFINE variable \"%s\" appears more than once", + restarget->name), + parser_errposition(pstate, exprLocation((Node *) inner_res))); + } + } + foreach_node(ResTarget, restarget, windef->rpCommonSyntax->rpDefs) { TargetEntry *teDefine; - - name = restarget->name; - - foreach_node(ResTarget, r, restargets) - { - char *n; - - n = r->name; - - if (!strcmp(n, name)) - ereport(ERROR, - errcode(ERRCODE_SYNTAX_ERROR), - errmsg("DEFINE variable \"%s\" appears more than once", - name), - parser_errposition(pstate, exprLocation((Node *) r))); - } - - restargets = lappend(restargets, restarget); + Node *expr; + List *vars; /* * Transform the DEFINE expression. We must NOT add the whole @@ -363,68 +365,56 @@ 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); + expr = coerce_to_boolean(pstate, expr, "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; - - foreach_node(TargetEntry, tle, *targetlist) - { - 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); - } - } - list_free(vars); - - /* Build the defineClause entry directly from the transformed expr */ - teDefine = makeTargetEntry((Expr *) expr, - list_length(defineClause) + 1, - pstrdup(name), - true); - } + /* Build the defineClause entry directly from the transformed expr */ + teDefine = makeTargetEntry((Expr *) expr, + list_length(defineClause) + 1, + pstrdup(restarget->name), + true); /* build transformed DEFINE clause (list of TargetEntry) */ defineClause = lappend(defineClause, teDefine); + + /* + * 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; + + foreach_node(TargetEntry, tle, *targetlist) + { + if (equal(tle->expr, var)) + { + found = true; + break; + } + } + + if (!found) + { + TargetEntry *newtle; + + newtle = makeTargetEntry((Expr *) copyObject(var), + list_length(*targetlist) + 1, + NULL, + true); + *targetlist = lappend(*targetlist, newtle); + } + } + list_free(vars); } - list_free(restargets); + pstate->p_rpr_pattern_vars = NIL; - /* - * Make sure that the row pattern definition search condition is a boolean - * expression. - */ - foreach_ptr(TargetEntry, te, defineClause) - te->expr = (Expr *) coerce_to_boolean(pstate, (Node *) te->expr, "DEFINE"); - /* * Validate DEFINE expressions: nested PREV/NEXT, column references, * compound flatten, volatile callees -- all in a single walk per diff --git a/src/test/regress/expected/rpr_base.out b/src/test/regress/expected/rpr_base.out index 41541898f5..cae5cab0d8 100644 --- a/src/test/regress/expected/rpr_base.out +++ b/src/test/regress/expected/rpr_base.out @@ -236,7 +236,7 @@ WINDOW w AS ( ); ERROR: DEFINE variable "a" appears more than once LINE 7: DEFINE A AS id > 0, A AS id < 10 - ^ + ^ DROP TABLE rpr_dup; -- Boolean coercion CREATE TABLE rpr_bool (id INT, flag BOOLEAN); -- 2.34.1