From d05704cb8025c06dbb577c27de7371142aa7f01f Mon Sep 17 00:00:00 2001 From: jian he Date: Sat, 17 Oct 2026 01:18:27 +0800 Subject: [PATCH v54 2/2] Remove compute_define_metadata compute_define_metadata() is complicated for what it computes: a context struct, a current-variable cursor, a separate classification function and an out-parameter, all of these is just to find which DEFINE variables depend on the match start. A simple expression walker can do this. Based on https://github.com/assam258-5892/postgres/tree/RPR-20260930 --- src/backend/executor/README.rpr | 2 +- src/backend/executor/execExpr.c | 5 - src/backend/optimizer/plan/createplan.c | 137 +++++++++--------------- src/tools/pgindent/typedefs.list | 1 - 4 files changed, 50 insertions(+), 95 deletions(-) diff --git a/src/backend/executor/README.rpr b/src/backend/executor/README.rpr index 4d5dadcc57..8b6b263308 100644 --- a/src/backend/executor/README.rpr +++ b/src/backend/executor/README.rpr @@ -324,7 +324,7 @@ LAST over any navigation, PREV or NEXT over PREV or NEXT, an inner navigation that is only part of the argument, as in PREV(v + FIRST(v)), and a third level. A navigation inside an offset is rejected as well. Once step (4) is done, no RPRNavExpr contains another one anywhere; the planner -relies on this (compute_matchStartDependent() asserts it), and the deparser +relies on this (contain_match_start_nav_walker() asserts it), and the deparser rebuilds the nested spelling from the compound kind. Variables that are used in PATTERN but not defined in DEFINE are implicitly diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c index 4bfe6ca95d..391348d216 100644 --- a/src/backend/executor/execExpr.c +++ b/src/backend/executor/execExpr.c @@ -5098,11 +5098,6 @@ ExecInitRPRNavExpr(ExprEvalStep *scratch, RPRNavExpr *nav, Assert(state->parent && IsA(state->parent, WindowAggState)); winstate = (WindowAggState *) state->parent; - /* - * Parser guarantee (mirrors compute_matchStartDependent): nav's direct - * children are never RPRNavExpr -- compound nesting is flattened in place - * and any other nesting is rejected. Outer-kind dispatch is sufficient. - */ Assert(nav->arg == NULL || !IsA(nav->arg, RPRNavExpr)); Assert(nav->offset_arg == NULL || !IsA(nav->offset_arg, RPRNavExpr)); Assert(nav->compound_offset_arg == NULL || diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c index ae510f9cc7..6991706791 100644 --- a/src/backend/optimizer/plan/createplan.c +++ b/src/backend/optimizer/plan/createplan.c @@ -2469,109 +2469,65 @@ create_minmaxagg_plan(PlannerInfo *root, MinMaxAggPath *best_path) return plan; } -/* - * DefineMetadataContext - context for the DEFINE clause walk below. - * - * The walk classifies one thing: which DEFINE variables depend on the match - * start, which is what buildRPRPattern() needs to decide context absorption. - * - * The driver sets curVarIdx to the index of the variable being walked before - * each invocation; the walker uses it to populate matchStartDependent. - */ -typedef struct DefineMetadataContext -{ - int curVarIdx; /* DEFINE variable currently being walked */ - Bitmapset *matchStartDependent; /* variables that depend on - * match_start */ -} DefineMetadataContext; - -/* - * compute_matchStartDependent - * - * per-variable match_start dependency for absorption suppression: outer nav - * kinds that reach match_start (FIRST, LAST-with-offset, PREV_FIRST, - * NEXT_FIRST, PREV_LAST/NEXT_LAST-with-offset) add curVarIdx to - * matchStartDependent. - * - * Classification uses only the outer nav kind: parser nesting restrictions - * prevent FIRST/LAST inside a PREV/NEXT value subexpression. - */ -static void -compute_matchStartDependent(RPRNavExpr *nav, DefineMetadataContext *context) -{ - /* - * Parser guarantee: by the time the planner sees a DEFINE expression, - * compound nesting has been flattened into a single RPRNavExpr and any - * other RPRNavExpr nesting has been rejected. So nav's direct child - * fields are not themselves RPRNavExpr nodes, and outer-kind dispatch - * below is sufficient. - */ - Assert(nav->arg == NULL || !IsA(nav->arg, RPRNavExpr)); - Assert(nav->offset_arg == NULL || !IsA(nav->offset_arg, RPRNavExpr)); - Assert(nav->compound_offset_arg == NULL || - !IsA(nav->compound_offset_arg, RPRNavExpr)); - - /* - * Match-start dependency: classify the outer nav kind. A constant - * LAST(x, 0) is conservatively included (offset_arg is a non-NULL Const), - * causing a harmless extra re-evaluation; since LAST(x, 0) is the current - * row, its result is independent of the match start. - */ - if (nav->kind == RPR_NAV_FIRST || - (nav->kind == RPR_NAV_LAST && nav->offset_arg != NULL) || - nav->kind == RPR_NAV_PREV_FIRST || - nav->kind == RPR_NAV_NEXT_FIRST || - ((nav->kind == RPR_NAV_PREV_LAST || - nav->kind == RPR_NAV_NEXT_LAST) && - nav->offset_arg != NULL)) - context->matchStartDependent = - bms_add_member(context->matchStartDependent, - context->curVarIdx); -} - static bool -define_metadata_walker(Node *node, DefineMetadataContext *ctx) +contain_match_start_nav_walker(Node *node, void *context) { if (node == NULL) return false; if (IsA(node, RPRNavExpr)) - compute_matchStartDependent(castNode(RPRNavExpr, node), ctx); + { + RPRNavExpr *nav = castNode(RPRNavExpr, node); + + /* + * Parser guarantee: by the time the planner sees a DEFINE expression, + * compound nesting has been flattened into a single RPRNavExpr and + * any other RPRNavExpr nesting has been rejected. So nav's direct + * child fields are not themselves RPRNavExpr nodes, and outer-kind + * dispatch below is sufficient. + */ + Assert(nav->arg == NULL || !IsA(nav->arg, RPRNavExpr)); + Assert(nav->offset_arg == NULL || !IsA(nav->offset_arg, RPRNavExpr)); + Assert(nav->compound_offset_arg == NULL || + !IsA(nav->compound_offset_arg, RPRNavExpr)); - return expression_tree_walker(node, define_metadata_walker, ctx); + /* + * Match-start dependency: classify the outer nav kind. A constant + * LAST(x, 0) is conservatively included (offset_arg is a non-NULL + * Const), causing a harmless extra re-evaluation; since LAST(x, 0) is + * the current row, its result is independent of the match start. + */ + if (nav->offset_arg != NULL) + { + if (nav->kind == RPR_NAV_PREV_LAST || + nav->kind == RPR_NAV_NEXT_LAST || + nav->kind == RPR_NAV_LAST) + return true; + } + + if (nav->kind == RPR_NAV_FIRST || + nav->kind == RPR_NAV_PREV_FIRST || + nav->kind == RPR_NAV_NEXT_FIRST) + return true; + } + + return expression_tree_walker(node, + contain_match_start_nav_walker, + context); } /* - * compute_define_metadata - * Classify which DEFINE variables depend on the match start. - * - * Walks each DEFINE variable expression once and returns the set of variable - * indices whose navigation reaches match_start: those containing FIRST or a - * compound PREV_FIRST/NEXT_FIRST, or a LAST that carries an offset of its - * own, whether plain or inside a compound PREV_LAST/NEXT_LAST. Such - * variables require per-context re-evaluation during NFA processing, and - * their presence disqualifies the pattern from context absorption. + * contain_match_start_nav + * Does the expression contain a navigation that depends on the match start? * * Navigation offsets for tuplestore trim are not computed here; the executor * records them as it compiles each navigation (ExecInitRPRNavExpr) and settles * them per scan (resolve_nav_offsets), which can evaluate non-constant * offsets that the planner cannot fold. */ -static void -compute_define_metadata(List *defineClause, Bitmapset **matchStartDependent) +static bool +contain_match_start_nav(Node *node) { - DefineMetadataContext ctx; - - ctx.curVarIdx = 0; - ctx.matchStartDependent = NULL; - - foreach_node(TargetEntry, te, defineClause) - { - ctx.curVarIdx = foreach_current_index(te); - - define_metadata_walker((Node *) te->expr, &ctx); - } - - *matchStartDependent = ctx.matchStartDependent; + return contain_match_start_nav_walker(node, NULL); } /* @@ -2658,7 +2614,12 @@ create_windowagg_plan(PlannerInfo *root, WindowAggPath *best_path) * absorption suppression in buildRPRPattern). Nav offsets for * tuplestore trim are built at executor init and resolved per scan. */ - compute_define_metadata(wc->defineClause, &matchStartDependent); + foreach_node(TargetEntry, te, wc->defineClause) + { + if (contain_match_start_nav((Node *) te->expr)) + matchStartDependent = bms_add_member(matchStartDependent, + foreach_current_index(te)); + } /* Compile and optimize RPR patterns */ compiledPattern = buildRPRPattern(wc->rpPattern, diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index 3ab9605230..709333f4d6 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -664,7 +664,6 @@ DecodingWorkerShared DefElem DefElemAction DefaultACLInfo -DefineMetadataContext DefinePhase DefineStmt DefineWalkCtx -- 2.34.1