nocfbot-0028-Clarify-RPR-comments-per-Jian-He-s-round-3-review.txt

From 24a92853516e24825f7c65cfc3c467cc401d659a Mon Sep 17 00:00:00 2001
From: Henson Choi <assam258@gmail.com>
Date: Mon, 1 Jun 2026 11:03:57 +0900
Subject: [PATCH 28/38] 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)