From 199dcd42cd5da6191af7b801b9fbe2f64e1d1bf6 Mon Sep 17 00:00:00 2001 From: Henson Choi Date: Tue, 7 Jul 2026 10:25:58 +0900 Subject: [PATCH] Clean up row pattern recognition internals Address several small, independent cleanups raised in Jian He's review of the row pattern recognition code: - Rename nfa_evaluate_row to rpr_evaluate_row. The function evaluates DEFINE variables at the RPR driver level in nodeWindowAgg.c; the nfa_ prefix belongs to the NFA engine internals in execRPR.c, and this was the sole nfa_-prefixed function outside that engine. - Remove the unused RPR_DEPTH_NONE macro (no code ever read it) and switch makeRPRPattern's array allocations to palloc_array/palloc0_array. - Pass currentPos into nfa_match so the current row index is visible in a debugger; it is the only NFA helper that otherwise lacks it. The parameter has no runtime consumer yet. Suggested-by: Jian He --- src/backend/executor/README.rpr | 18 +++++++++--------- src/backend/executor/execRPR.c | 17 +++++++++++------ src/backend/executor/nodeWindowAgg.c | 10 +++++----- src/backend/optimizer/plan/rpr.c | 6 +++--- src/include/optimizer/rpr.h | 1 - 5 files changed, 28 insertions(+), 24 deletions(-) diff --git a/src/backend/executor/README.rpr b/src/backend/executor/README.rpr index 9275e265d4b..d4c53fd3f63 100644 --- a/src/backend/executor/README.rpr +++ b/src/backend/executor/README.rpr @@ -564,7 +564,7 @@ Pseudocode of the row processing loop: targetCtx = ExecRPRStartContext(pos) for currentPos = startPos; targetCtx->states != NULL; currentPos++: - if not nfa_evaluate_row(currentPos): -- row does not exist + if not rpr_evaluate_row(currentPos): -- row does not exist ExecRPRFinalizeAllContexts() -- finalize all contexts ExecRPRCleanupDeadContexts() -- clean up after finalization break @@ -601,7 +601,7 @@ the pattern. For example, the initial advance for PATTERN ((A | B) C): During the initial advance, reaching FIN is not recorded as a match. This is to prevent empty matches. -VI-3. Row Evaluation: nfa_evaluate_row() +VI-3. Row Evaluation: rpr_evaluate_row() Evaluates all variable conditions in the DEFINE clause at once for the current row. @@ -638,7 +638,7 @@ VI-4. Per-Context Re-evaluation (match_start_dependent variables) DEFINE variables that depend on match_start (those containing FIRST, LAST-with-offset, or compound PREV_FIRST/NEXT_FIRST/PREV_LAST/NEXT_LAST) are identified at plan time via defineMatchStartDependent. The shared -evaluation in nfa_evaluate_row() uses the head context's matchStartRow +evaluation in rpr_evaluate_row() uses the head context's matchStartRow for FIRST/LAST base position. When processing a context whose matchStartRow differs from the shared @@ -1192,7 +1192,7 @@ XI-4. Execution Trace Initial advance: elemIdx=0(A) -> VAR, so state is added. C0.states = [{elemIdx=0, counts=[0]}] - nfa_evaluate_row(0): + rpr_evaluate_row(0): A: price(100) > PREV(price) -> no PREV -> false B: price(100) < PREV(price) -> no PREV -> false varMatched = [false, false] @@ -1214,7 +1214,7 @@ XI-4. Execution Trace Context C1 created (matchStartRow=1). Initial advance: C1.states = [{elemIdx=0, counts=[0]}] - nfa_evaluate_row(1): + rpr_evaluate_row(1): A: 110 > PREV(100) -> true B: 110 < PREV(100) -> false varMatched = [true, false] @@ -1236,7 +1236,7 @@ XI-4. Execution Trace Context C2 created (matchStartRow=2). Initial advance: C2.states = [{elemIdx=0, counts=[0]}] - nfa_evaluate_row(2): + rpr_evaluate_row(2): A: 120 > PREV(110) -> true B: 120 < PREV(110) -> false varMatched = [true, false] @@ -1268,7 +1268,7 @@ XI-4. Execution Trace --- Row 3 (price=115) --- - nfa_evaluate_row(3): + rpr_evaluate_row(3): A: 115 > PREV(120) -> false B: 115 < PREV(120) -> true varMatched = [false, true] @@ -1297,7 +1297,7 @@ XI-4. Execution Trace C3 was already created but matchStartRow=3, so it is not applicable. New context C4 created (matchStartRow=4). - nfa_evaluate_row(4): + rpr_evaluate_row(4): A: 130 > PREV(115) -> true B: 130 < PREV(115) -> false @@ -1473,7 +1473,7 @@ Appendix A. Key Function Index finalizeRPRPattern rpr.c Finalization computeAbsorbability rpr.c Absorption analysis update_reduced_frame nodeWindowAgg.c Execution main loop - nfa_evaluate_row nodeWindowAgg.c DEFINE evaluation + rpr_evaluate_row nodeWindowAgg.c DEFINE evaluation ExecRPRStartContext execRPR.c Context creation ExecRPRProcessRow execRPR.c 3-phase processing nfa_match execRPR.c Phase 1 diff --git a/src/backend/executor/execRPR.c b/src/backend/executor/execRPR.c index a85ab563abb..0127ae82925 100644 --- a/src/backend/executor/execRPR.c +++ b/src/backend/executor/execRPR.c @@ -82,7 +82,7 @@ static void nfa_absorb_contexts(WindowAggState *winstate); static bool nfa_eval_var_match(WindowAggState *winstate, RPRPatternElement *elem, bool *varMatched); static void nfa_match(WindowAggState *winstate, RPRNFAContext *ctx, - bool *varMatched); + bool *varMatched, int64 currentPos); static void nfa_route_to_elem(WindowAggState *winstate, RPRNFAContext *ctx, RPRNFAState *state, RPRPatternElement *nextElem, int64 currentPos); @@ -793,9 +793,14 @@ nfa_eval_var_match(WindowAggState *winstate, RPRPatternElement *elem, * - Chains through END elements while count >= max (must-exit path) * * Non-VAR elements (ALT, END, FIN) are kept as-is for advance phase. + * + * currentPos is threaded in only for debugging visibility (nfa_match is the + * one NFA helper that otherwise lacks the row index); it has no runtime + * consumer yet. */ static void -nfa_match(WindowAggState *winstate, RPRNFAContext *ctx, bool *varMatched) +nfa_match(WindowAggState *winstate, RPRNFAContext *ctx, bool *varMatched, + int64 currentPos pg_attribute_unused()) { RPRPattern *pattern = winstate->rpPattern; RPRPatternElement *elements = pattern->elements; @@ -1595,7 +1600,7 @@ nfa_advance(WindowAggState *winstate, RPRNFAContext *ctx, int64 currentPos) * * Only variables in defineMatchStartDependent are re-evaluated. The * current row's slot (ecxt_outertuple) must already be set up by - * nfa_evaluate_row(). + * rpr_evaluate_row(). */ static void nfa_reevaluate_dependent_vars(WindowAggState *winstate, RPRNFAContext *ctx, @@ -1842,7 +1847,7 @@ ExecRPRProcessRow(WindowAggState *winstate, int64 currentPos, if (currentPos == ctxFrameEnd) { /* Frame boundary reached: force mismatch */ - nfa_match(winstate, ctx, NULL); + nfa_match(winstate, ctx, NULL, currentPos); continue; } } @@ -1854,7 +1859,7 @@ ExecRPRProcessRow(WindowAggState *winstate, int64 currentPos, */ if (hasDependent && ctx->matchStartRow != winstate->nav_match_start) nfa_reevaluate_dependent_vars(winstate, ctx, currentPos); - nfa_match(winstate, ctx, varMatched); + nfa_match(winstate, ctx, varMatched, currentPos); ctx->lastProcessedRow = currentPos; } @@ -1987,7 +1992,7 @@ ExecRPRFinalizeAllContexts(WindowAggState *winstate, int64 lastPos) if (ctx->states != NULL) { - nfa_match(winstate, ctx, NULL); + nfa_match(winstate, ctx, NULL, lastPos); nfa_advance(winstate, ctx, lastPos); } } diff --git a/src/backend/executor/nodeWindowAgg.c b/src/backend/executor/nodeWindowAgg.c index 9ffca036953..0ca09c8d4ad 100644 --- a/src/backend/executor/nodeWindowAgg.c +++ b/src/backend/executor/nodeWindowAgg.c @@ -246,8 +246,8 @@ static void advance_reduced_frame_nfa(WindowObject winobj, bool hasLimitedFrame, int64 frameOffset); static void update_reduced_frame(WindowObject winobj, int64 pos); -/* Forward declarations - NFA row evaluation */ -static bool nfa_evaluate_row(WindowObject winobj, int64 pos, bool *varMatched); +/* Forward declarations - DEFINE row evaluation */ +static bool rpr_evaluate_row(WindowObject winobj, int64 pos, bool *varMatched); /* Forward declarations - navigation offset evaluation */ static void eval_define_offsets(WindowAggState *winstate, List *defineClause); @@ -4474,7 +4474,7 @@ advance_reduced_frame_nfa(WindowObject winobj, RPRNFAContext *targetCtx, * when matchStartRow differs. */ winstate->nav_match_start = targetCtx->matchStartRow; - rowExists = nfa_evaluate_row(winobj, currentPos, winstate->nfaVarMatched); + rowExists = rpr_evaluate_row(winobj, currentPos, winstate->nfaVarMatched); /* No more rows in partition? Finalize all contexts */ if (!rowExists) @@ -4645,7 +4645,7 @@ register_result: } /* - * nfa_evaluate_row + * rpr_evaluate_row * * Evaluate all DEFINE variables for current row. * Returns true if the row exists, false if out of partition. @@ -4657,7 +4657,7 @@ register_result: * opcodes during expression evaluation, which temporarily swap the slot. */ static bool -nfa_evaluate_row(WindowObject winobj, int64 pos, bool *varMatched) +rpr_evaluate_row(WindowObject winobj, int64 pos, bool *varMatched) { WindowAggState *winstate = winobj->winstate; ExprContext *econtext = winstate->rprContext; diff --git a/src/backend/optimizer/plan/rpr.c b/src/backend/optimizer/plan/rpr.c index fac3d7bcf38..da918a908a7 100644 --- a/src/backend/optimizer/plan/rpr.c +++ b/src/backend/optimizer/plan/rpr.c @@ -1090,20 +1090,20 @@ makeRPRPattern(int numVars, int numElements, RPRDepth maxDepth, result = makeNode(RPRPattern); result->numVars = numVars; - /* depth < RPR_DEPTH_MAX, so maxDepth+1 never aliases RPR_DEPTH_NONE. */ + /* depth < RPR_DEPTH_MAX, so maxDepth + 1 does not exceed RPR_DEPTH_MAX. */ Assert(maxDepth < RPR_DEPTH_MAX); result->maxDepth = maxDepth + 1; /* +1: depth is 0-based */ result->numElements = numElements; /* Copy varNames (pattern must have at least one variable) */ Assert(numVars > 0); - result->varNames = palloc(numVars * sizeof(char *)); + result->varNames = palloc_array(char *, numVars); for (i = 0; i < numVars; i++) result->varNames[i] = pstrdup(varNamesStack[i]); /* Allocate elements array (zero-init for reserved fields) */ Assert(numElements >= 2); - result->elements = palloc0(numElements * sizeof(RPRPatternElement)); + result->elements = palloc0_array(RPRPatternElement, numElements); return result; } diff --git a/src/include/optimizer/rpr.h b/src/include/optimizer/rpr.h index 1de7f86f867..d73f8c8e693 100644 --- a/src/include/optimizer/rpr.h +++ b/src/include/optimizer/rpr.h @@ -40,7 +40,6 @@ #define RPR_ELEMIDX_INVALID ((RPRElemIdx) -1) /* invalid index */ #define RPR_DEPTH_MAX (PG_UINT8_MAX - 1) /* max pattern nesting depth: * 254 */ -#define RPR_DEPTH_NONE PG_UINT8_MAX /* no enclosing group (top-level) */ /* Reserved control-element varIds (high nibble 0xF; 0xF0-0xFB spare) */ #define RPR_VARID_BEGIN ((RPRVarId) 0xFC) /* group begin */