From 93f98d31fc2a9ca58cd973b671b5b241c6d5d461 Mon Sep 17 00:00:00 2001 From: Henson Choi Date: Tue, 24 Feb 2026 12:38:05 +0900 Subject: [PATCH 8/8] Remove bare variable-scoping blocks in RPR code --- src/backend/optimizer/plan/rpr.c | 66 ++++++++++++++-------------- src/backend/parser/parse_rpr.c | 74 ++++++++++++++++---------------- 2 files changed, 68 insertions(+), 72 deletions(-) diff --git a/src/backend/optimizer/plan/rpr.c b/src/backend/optimizer/plan/rpr.c index 987f595d434..4414b71b5ec 100644 --- a/src/backend/optimizer/plan/rpr.c +++ b/src/backend/optimizer/plan/rpr.c @@ -503,6 +503,7 @@ mergeGroupPrefixSuffix(List *children) List *groupContent = child->children; int groupChildCount; int prefixLen = list_length(result); + List *trimmed; /* * If GROUP has single SEQ child, compare with SEQ's children. @@ -546,17 +547,14 @@ mergeGroupPrefixSuffix(List *children) child->min += 1; /* Rebuild result without matched prefix */ + trimmed = NIL; + for (j = 0; j < prefixLen - groupChildCount; j++) { - List *trimmed = NIL; - - for (j = 0; j < prefixLen - groupChildCount; j++) - { - trimmed = lappend(trimmed, - list_nth(result, j)); - } - result = trimmed; - prefixLen = list_length(result); + trimmed = lappend(trimmed, + list_nth(result, j)); } + result = trimmed; + prefixLen = list_length(result); } else { @@ -1226,9 +1224,11 @@ static void fillRPRPatternAlt(RPRPatternNode *node, RPRPattern *pat, int *idx, RPRDepth depth) { ListCell *lc; + ListCell *lc2; RPRPatternElement *elem; List *altBranchStarts = NIL; List *altEndPositions = NIL; + int afterAltIdx; /* Add alternation start marker */ elem = &pat->elements[*idx]; @@ -1262,38 +1262,36 @@ fillRPRPatternAlt(RPRPatternNode *node, RPRPattern *pat, int *idx, RPRDepth dept } /* Set next on last element of each alternative to after the alternation */ + afterAltIdx = *idx; + lc2 = list_head(altBranchStarts); + + foreach(lc, altEndPositions) { - int afterAltIdx = *idx; - ListCell *lc2 = list_head(altBranchStarts); + int endPos = lfirst_int(lc); + int branchStart = lfirst_int(lc2); - foreach(lc, altEndPositions) + if (pat->elements[endPos].next != RPR_ELEMIDX_INVALID) { - int endPos = lfirst_int(lc); - int branchStart = lfirst_int(lc2); - - if (pat->elements[endPos].next != RPR_ELEMIDX_INVALID) - { - /* - * An inner ALT already set next on this element. Redirect - * all elements in this branch that share the same target to - * point to after this ALT instead. - */ - int oldTarget = pat->elements[endPos].next; - int j; + /* + * An inner ALT already set next on this element. Redirect all + * elements in this branch that share the same target to point to + * after this ALT instead. + */ + int oldTarget = pat->elements[endPos].next; + int j; - for (j = branchStart; j <= endPos; j++) - { - if (pat->elements[j].next == oldTarget) - pat->elements[j].next = afterAltIdx; - } - } - else + for (j = branchStart; j <= endPos; j++) { - pat->elements[endPos].next = afterAltIdx; + if (pat->elements[j].next == oldTarget) + pat->elements[j].next = afterAltIdx; } - - lc2 = lnext(altBranchStarts, lc2); } + else + { + pat->elements[endPos].next = afterAltIdx; + } + + lc2 = lnext(altBranchStarts, lc2); } list_free(altBranchStarts); diff --git a/src/backend/parser/parse_rpr.c b/src/backend/parser/parse_rpr.c index 9e1fa228759..027a3d2500a 100644 --- a/src/backend/parser/parse_rpr.c +++ b/src/backend/parser/parse_rpr.c @@ -267,6 +267,7 @@ transformDefineClause(ParseState *pstate, WindowClause *wc, WindowDef *windef, { TargetEntry *te, *teDefine; + int defineExprLocation; restarget = (ResTarget *) lfirst(lc); name = restarget->name; @@ -306,44 +307,41 @@ transformDefineClause(ParseState *pstate, WindowClause *wc, WindowDef *windef, * BY), not the DEFINE clause. We need to preserve the DEFINE location * for accurate error reporting. */ - { - int defineExprLocation = exprLocation(restarget->val); - - te = findTargetlistEntrySQL99(pstate, restarget->val, - targetlist, EXPR_KIND_RPR_DEFINE); - - /* ------------------- - * Copy the TargetEntry for defineClause and always set the pattern - * variable name. We use copyObject so the original targetlist entry - * is not modified. - * - * Note: We must always set resname to the pattern variable name. - * findTargetlistEntrySQL99 creates new TEs with resname = NULL - * (resjunk entries), but returns existing TEs unchanged when the - * expression already exists in targetlist. - * - * Example: "SELECT id, flag, ... WINDOW w AS (... DEFINE T AS flag)" - * - * 1. SELECT list processing creates: TE{resname="flag", expr=flag} - * 2. DEFINE T AS flag: findTargetlistEntrySQL99 finds existing TE - * 3. te->resname is "flag" (from SELECT), not NULL - * 4. Without unconditionally setting resname, teDefine->resname - * would remain "flag" instead of pattern variable name "T" - * 5. buildRPRPattern builds defineVariableList from resname, so - * it would contain ["flag"] instead of ["T"] - * 6. Pattern variable "T" not found -> Assert failure crash - */ - teDefine = copyObject(te); - teDefine->resname = pstrdup(name); - - /* - * Update the expression location to point to the DEFINE clause. - * This ensures error messages reference the correct source - * location. - */ - if (defineExprLocation >= 0 && IsA(teDefine->expr, Var)) - ((Var *) teDefine->expr)->location = defineExprLocation; - } + defineExprLocation = exprLocation(restarget->val); + + te = findTargetlistEntrySQL99(pstate, restarget->val, + targetlist, EXPR_KIND_RPR_DEFINE); + + /* ------------------- + * Copy the TargetEntry for defineClause and always set the pattern + * variable name. We use copyObject so the original targetlist entry + * is not modified. + * + * Note: We must always set resname to the pattern variable name. + * findTargetlistEntrySQL99 creates new TEs with resname = NULL + * (resjunk entries), but returns existing TEs unchanged when the + * expression already exists in targetlist. + * + * Example: "SELECT id, flag, ... WINDOW w AS (... DEFINE T AS flag)" + * + * 1. SELECT list processing creates: TE{resname="flag", expr=flag} + * 2. DEFINE T AS flag: findTargetlistEntrySQL99 finds existing TE + * 3. te->resname is "flag" (from SELECT), not NULL + * 4. Without unconditionally setting resname, teDefine->resname + * would remain "flag" instead of pattern variable name "T" + * 5. buildRPRPattern builds defineVariableList from resname, so + * it would contain ["flag"] instead of ["T"] + * 6. Pattern variable "T" not found -> Assert failure crash + */ + teDefine = copyObject(te); + teDefine->resname = pstrdup(name); + + /* + * Update the expression location to point to the DEFINE clause. This + * ensures error messages reference the correct source location. + */ + if (defineExprLocation >= 0 && IsA(teDefine->expr, Var)) + ((Var *) teDefine->expr)->location = defineExprLocation; /* build transformed DEFINE clause (list of TargetEntry) */ defineClause = lappend(defineClause, teDefine); -- 2.50.1 (Apple Git-155)