From 686c593a23d5604e48a5f08a84cf525cbda8a072 Mon Sep 17 00:00:00 2001 From: Henson Choi Date: Mon, 1 Jun 2026 11:03:57 +0900 Subject: [PATCH 28/68] Clarify RPR comments per Jian He's round-3 review - Replace fragile "Tests line NNN" comments with the branch and condition they exercise, so they survive later code shuffles - Reword nfa_add_state_unique to describe appending to the list tail - Disambiguate "END chain" as the chain of END elements - Restate the nfa_advance_var Assert comment as a structural invariant (min <= max), not a post-match claim --- src/backend/executor/execRPR.c | 11 ++++++----- src/test/regress/expected/rpr_base.out | 8 ++++---- src/test/regress/sql/rpr_base.sql | 8 ++++---- 3 files changed, 14 insertions(+), 13 deletions(-) diff --git a/src/backend/executor/execRPR.c b/src/backend/executor/execRPR.c index 4463cfe0a5c..9bad36239f8 100644 --- a/src/backend/executor/execRPR.c +++ b/src/backend/executor/execRPR.c @@ -334,7 +334,8 @@ nfa_states_equal(WindowAggState *winstate, RPRNFAState *s1, RPRNFAState *s2) /* * nfa_add_state_unique * - * Add a state to ctx->states at the END, only if no duplicate exists. + * Add the state to the end of the ctx->states linked list, but only if a + * duplicate state is not already present. * Earlier states have better lexical order (DFS traversal order), so existing * wins; the new state is freed when a duplicate is found. */ @@ -779,7 +780,7 @@ nfa_eval_var_match(WindowAggState *winstate, RPRPatternElement *elem, * previous advance when count >= min was satisfied) * * For VARs that reached max count followed by END: - * - Advance through END chain to reach absorption judgment point + * - Advance through the END-element chain to the absorption judgment point * - Only deterministic exits (count >= max, max != INF) are handled * - Chains through END elements while count >= max (must-exit path) * @@ -796,8 +797,8 @@ nfa_match(WindowAggState *winstate, RPRNFAContext *ctx, bool *varMatched) /* * Evaluate VAR elements against current row. For VARs that reach max - * count with END next, advance through END chain inline so absorb phase - * can compare states at judgment points. + * count with END next, advance through the chain of END elements inline + * so absorb phase can compare states at judgment points. */ for (state = ctx->states; state != NULL; state = nextState) { @@ -1229,7 +1230,7 @@ nfa_advance_var(WindowAggState *winstate, RPRNFAContext *ctx, bool canLoop = (elem->max == RPR_QUANTITY_INF || count < elem->max); bool canExit = (count >= elem->min); - /* After a successful match, count >= 1, so at least one must be true */ + /* min <= max, so !canExit (count < min) implies canLoop (count < max) */ Assert(canLoop || canExit); /* elem->next must be a valid index for any reachable VAR */ diff --git a/src/test/regress/expected/rpr_base.out b/src/test/regress/expected/rpr_base.out index d8f805c89aa..c96a1216cc3 100644 --- a/src/test/regress/expected/rpr_base.out +++ b/src/test/regress/expected/rpr_base.out @@ -3442,8 +3442,8 @@ WINDOW w AS (ORDER BY id ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING (7 rows) -- Consecutive VAR merge: A A+ -> a{2,} --- Tests line 251: child->max == RPR_QUANTITY_INF branch in mergeConsecutiveVars --- prev: A{1,1} (finite), child: A+ (infinite) triggers line 251 evaluation +-- Exercises the child->max == RPR_QUANTITY_INF branch in mergeConsecutiveVars, +-- where a finite prev (A{1,1}) meets an infinite child (A+). EXPLAIN (COSTS OFF) SELECT COUNT(*) OVER w FROM rpr_plan WINDOW w AS (ORDER BY id ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING @@ -3492,8 +3492,8 @@ WINDOW w AS (ORDER BY id ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING (7 rows) -- Consecutive GROUP merge: (A B){2} (A B)+ -> (a b){3,} --- Tests line 325: child->max == RPR_QUANTITY_INF branch in mergeConsecutiveGroups --- prev: (A B){2,2} (finite), child: (A B)+ (infinite) triggers line 325 evaluation +-- Exercises the child->max == RPR_QUANTITY_INF branch in mergeConsecutiveGroups, +-- where a finite prev ((A B){2,2}) meets an infinite child ((A B)+). EXPLAIN (COSTS OFF) SELECT COUNT(*) OVER w FROM rpr_plan WINDOW w AS (ORDER BY id ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING diff --git a/src/test/regress/sql/rpr_base.sql b/src/test/regress/sql/rpr_base.sql index 6c2365a2d20..d7b63cfc690 100644 --- a/src/test/regress/sql/rpr_base.sql +++ b/src/test/regress/sql/rpr_base.sql @@ -2379,8 +2379,8 @@ WINDOW w AS (ORDER BY id ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING PATTERN (A+ A*) DEFINE A AS val > 0); -- Consecutive VAR merge: A A+ -> a{2,} --- Tests line 251: child->max == RPR_QUANTITY_INF branch in mergeConsecutiveVars --- prev: A{1,1} (finite), child: A+ (infinite) triggers line 251 evaluation +-- Exercises the child->max == RPR_QUANTITY_INF branch in mergeConsecutiveVars, +-- where a finite prev (A{1,1}) meets an infinite child (A+). EXPLAIN (COSTS OFF) SELECT COUNT(*) OVER w FROM rpr_plan WINDOW w AS (ORDER BY id ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING @@ -2399,8 +2399,8 @@ WINDOW w AS (ORDER BY id ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING PATTERN ((A B)+ (A B)+) DEFINE A AS val <= 50, B AS val > 50); -- Consecutive GROUP merge: (A B){2} (A B)+ -> (a b){3,} --- Tests line 325: child->max == RPR_QUANTITY_INF branch in mergeConsecutiveGroups --- prev: (A B){2,2} (finite), child: (A B)+ (infinite) triggers line 325 evaluation +-- Exercises the child->max == RPR_QUANTITY_INF branch in mergeConsecutiveGroups, +-- where a finite prev ((A B){2,2}) meets an infinite child ((A B)+). EXPLAIN (COSTS OFF) SELECT COUNT(*) OVER w FROM rpr_plan WINDOW w AS (ORDER BY id ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING -- 2.50.1 (Apple Git-155)