From d598f7bc61e2fc828ff5a92cf16fc000284bf492 Mon Sep 17 00:00:00 2001 From: Henson Choi Date: Mon, 22 Jun 2026 23:28:53 +0900 Subject: [PATCH v50 24/29] Simplify row pattern compilation by passing the WindowClause buildRPRPattern() took rpPattern, rpSkipTo, frameOptions, and a pre-built defineVariableList as separate arguments. Pass the WindowClause directly instead: it carries all of these, and buildRPRPattern can walk wc->defineClause itself to collect the DEFINE variable names. This removes the intermediate List that create_windowagg_plan built only to hand off, along with the collectDefineVariables() helper that unpacked it. Also inline tryUnwrapSingleChild() into its two callers (optimizeSeqPattern and optimizeAltPattern); it was a trivial one-liner. Based on a patch by Jian He. --- src/backend/optimizer/plan/createplan.c | 18 +---- src/backend/optimizer/plan/rpr.c | 92 +++++++++---------------- src/include/optimizer/rpr.h | 4 +- src/include/parser/parse_node.h | 2 +- 4 files changed, 37 insertions(+), 79 deletions(-) diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c index d96e76b0221..d860fb9711e 100644 --- a/src/backend/optimizer/plan/createplan.c +++ b/src/backend/optimizer/plan/createplan.c @@ -2820,7 +2820,6 @@ create_windowagg_plan(PlannerInfo *root, WindowAggPath *best_path) Oid *ordOperators; Oid *ordCollations; ListCell *lc; - List *defineVariableList = NIL; RPRPattern *compiledPattern = NULL; Bitmapset *matchStartDependent = NULL; RPRNavOffsetKind navMaxOffsetKind = RPR_NAV_OFFSET_FIXED; @@ -2879,18 +2878,9 @@ create_windowagg_plan(PlannerInfo *root, WindowAggPath *best_path) ordNumCols++; } - /* Build RPR pattern and defineVariableList */ + /* Build RPR pattern */ if (wc->rpPattern) { - /* - * Build defineVariableList from defineClause. The parser already - * rejects DEFINE variables not used in PATTERN, so no filtering is - * needed. - */ - foreach_node(TargetEntry, te, wc->defineClause) - defineVariableList = lappend(defineVariableList, - makeString(pstrdup(te->resname))); - /* * Walk DEFINE once: collect nav offsets (for tuplestore trim) and the * bitmapset of match_start-dependent variables (for absorption @@ -2903,11 +2893,7 @@ create_windowagg_plan(PlannerInfo *root, WindowAggPath *best_path) &matchStartDependent); /* Compile and optimize RPR patterns */ - compiledPattern = buildRPRPattern(wc->rpPattern, - defineVariableList, - wc->rpSkipTo, - wc->frameOptions, - !bms_is_empty(matchStartDependent)); + compiledPattern = buildRPRPattern(wc, !bms_is_empty(matchStartDependent)); } /* And finally we can make the WindowAgg node */ diff --git a/src/backend/optimizer/plan/rpr.c b/src/backend/optimizer/plan/rpr.c index 3d7752bd5ed..1c59e8a70a0 100644 --- a/src/backend/optimizer/plan/rpr.c +++ b/src/backend/optimizer/plan/rpr.c @@ -48,9 +48,6 @@ /* Forward declarations */ static bool rprPatternEqual(RPRPatternNode *a, RPRPatternNode *b); static bool rprPatternChildrenEqual(List *a, List *b); - -static RPRPatternNode *tryUnwrapSingleChild(RPRPatternNode *pattern); - static List *flattenSeqChildren(List *children); static List *mergeConsecutiveVars(List *children); static List *mergeConsecutiveGroups(List *children); @@ -67,8 +64,6 @@ static RPRPatternNode *tryUnwrapGroup(RPRPatternNode *pattern); static RPRPatternNode *optimizeGroupPattern(RPRPatternNode *pattern); static RPRPatternNode *optimizeRPRPattern(RPRPatternNode *pattern); - -static int collectDefineVariables(List *defineVariableList, char **varNames); static void scanRPRPatternRecursive(RPRPatternNode *node, char **varNames, int *numVars, int *numElements, RPRDepth depth, RPRDepth *maxDepth); @@ -155,27 +150,6 @@ rprPatternChildrenEqual(List *a, List *b) return true; } -/* - * tryUnwrapSingleChild - * Try to unwrap pattern node with single child. - * - * Examples (internal node representation): - * SEQ[A] -> A (single-element sequence becomes the element) - * ALT[A] -> A (single-alternative becomes the alternative) - * - * If pattern has exactly one child, return the child directly. - * Otherwise returns the pattern unchanged. - * Used by both SEQ and ALT optimization. - */ -static RPRPatternNode * -tryUnwrapSingleChild(RPRPatternNode *pattern) -{ - if (list_length(pattern->children) == 1) - return (RPRPatternNode *) linitial(pattern->children); - - return pattern; -} - /* * flattenSeqChildren * Recursively optimize children and flatten nested SEQ. @@ -669,8 +643,11 @@ optimizeSeqPattern(RPRPatternNode *pattern) /* Merge prefix/suffix into GROUP with matching children */ pattern->children = mergeGroupPrefixSuffix(pattern->children); - /* Unwrap single-item SEQ */ - return tryUnwrapSingleChild(pattern); + /* Unwrap single-item SEQ: SEQ[A] -> A */ + if (list_length(pattern->children) == 1) + return (RPRPatternNode *) linitial(pattern->children); + + return pattern; } /* @@ -754,8 +731,11 @@ optimizeAltPattern(RPRPatternNode *pattern) /* Remove duplicate alternatives */ pattern->children = removeDuplicateAlternatives(pattern->children); - /* Unwrap single-item ALT */ - return tryUnwrapSingleChild(pattern); + /* Unwrap single-item ALT: ALT[A] -> A */ + if (list_length(pattern->children) == 1) + return (RPRPatternNode *) linitial(pattern->children); + + return pattern; } /* @@ -969,30 +949,6 @@ optimizeRPRPattern(RPRPatternNode *pattern) return pattern; } -/* - * collectDefineVariables - * Collect variable names from DEFINE clause. - * - * Populates varNames array with variable names in DEFINE order. - * This ensures varId == defineIdx, eliminating runtime mapping. - * Returns the number of variables collected. - */ -static int -collectDefineVariables(List *defineVariableList, char **varNames) -{ - int numVars = 0; - - foreach_node(String, varname, defineVariableList) - { - /* Parser already checked this limit in transformDefineClause */ - Assert(numVars <= RPR_VARID_MAX); - - varNames[numVars++] = strVal(varname); - } - - return numVars; -} - /* * scanRPRPatternRecursive * Recursively scan pattern parse tree (pass 1 internal). @@ -1904,9 +1860,7 @@ validate_rpr_define_volatility(List *defineClause) * Called from createplan.c during plan creation. */ RPRPattern * -buildRPRPattern(RPRPatternNode *pattern, List *defineVariableList, - RPSkipTo rpSkipTo, int frameOptions, - bool hasMatchStartDependent) +buildRPRPattern(WindowClause *wc, bool hasMatchStartDependent) { RPRPattern *result; RPRPatternNode *optimized; @@ -1915,6 +1869,9 @@ buildRPRPattern(RPRPatternNode *pattern, List *defineVariableList, int numElements; RPRDepth maxDepth; int idx; + RPRPatternNode *pattern = wc->rpPattern; + RPSkipTo rpSkipTo = wc->rpSkipTo; + int frameOptions = wc->frameOptions; /* Caller must check for NULL pattern before calling */ Assert(pattern != NULL); @@ -1924,12 +1881,29 @@ buildRPRPattern(RPRPatternNode *pattern, List *defineVariableList, /* Optimize the pattern tree */ optimized = optimizeRPRPattern(copyObject(pattern)); - /* Collect variable names from DEFINE clause */ - numVars = collectDefineVariables(defineVariableList, varNamesStack); + numVars = 0; + + /* + * Populate varNamesStack with the DEFINE variable names in DEFINE order. + * This ensures varId == defineClause index, eliminating runtime mapping. + */ + foreach_node(TargetEntry, te, wc->defineClause) + { + /* Parser always assigns a name to each DEFINE entry */ + Assert(te->resname != NULL); + + varNamesStack[numVars++] = pstrdup(te->resname); + } /* Scan pattern: collect variables, count elements, validate limits */ scanRPRPattern(optimized, varNamesStack, &numVars, &numElements, &maxDepth); + /* + * numVars may reach RPR_VARID_MAX + 1 (valid varIds are 0 .. + * RPR_VARID_MAX) + */ + Assert(numVars <= RPR_VARID_MAX + 1); + /* Allocate result structure */ result = makeRPRPattern(numVars, numElements, maxDepth, varNamesStack); diff --git a/src/include/optimizer/rpr.h b/src/include/optimizer/rpr.h index f5462ab2a7c..1de7f86f867 100644 --- a/src/include/optimizer/rpr.h +++ b/src/include/optimizer/rpr.h @@ -75,9 +75,7 @@ #define RPRElemCanSkip(e) ((e)->min == 0) extern void validate_rpr_define_volatility(List *defineClause); -extern RPRPattern *buildRPRPattern(RPRPatternNode *pattern, List *defineVariableList, - RPSkipTo rpSkipTo, int frameOptions, - bool hasMatchStartDependent); +extern RPRPattern *buildRPRPattern(WindowClause *wc, bool hasMatchStartDependent); /* * Shared traversal walker for DEFINE clause RPRNavExpr collection. diff --git a/src/include/parser/parse_node.h b/src/include/parser/parse_node.h index cb9d02c00c4..78c6ca9fb5b 100644 --- a/src/include/parser/parse_node.h +++ b/src/include/parser/parse_node.h @@ -231,7 +231,7 @@ struct ParseState ParseNamespaceItem *p_grouping_nsitem; /* NSItem for grouping, or NULL */ List *p_windowdefs; /* raw representations of window clauses */ ParseExprKind p_expr_kind; /* what kind of expression we're parsing */ - List *p_rpr_pattern_vars; /* RPR variable names for DEFINE clause */ + List *p_rpr_pattern_vars; /* Row pattern variable names */ int p_next_resno; /* next targetlist resno to assign */ List *p_multiassign_exprs; /* junk tlist entries for multiassign */ List *p_locking_clause; /* raw FOR UPDATE/FOR SHARE info */ -- 2.50.1 (Apple Git-155)