From 0cb1aeacd82cf06f6fb5d5fb2d4c8597a9e257c1 Mon Sep 17 00:00:00 2001 From: jian he Date: Sun, 30 Aug 2026 20:52:24 +0900 Subject: [PATCH] Name the RPR quantifier tests instead of spelling them out nfa_advance_var() and nfa_advance_end() made the same quantifier decisions, one through local booleans and the other written out by hand. Both already guarded the unbounded case with an explicit RPR_QUANTITY_INF test, in complementary forms that had to be read side by side to be seen as one rule. Put the tests in rpr.h so both callers ask the same question: RPRElemIsUnbounded(e) max is the unbounded sentinel RPRElemCanLoop(e, count) may iterate once more RPRElemCanExit(e, count) has reached the lower bound RPRElemWithinMax(e, count) has not passed the upper bound The last two differ only in < versus <=, and the raw form of one sat next to the other in three assertions, which invites "simplifying" them into one. Naming both removes the invitation. Name the saturating increment too. Six places repeated the same guarded ++, three of them writing the array subscript twice. nfa_match() keeps its bare count >= max on purpose, and the pattern-tree half of optimizer/plan/rpr.c works on RPRPatternNode, so the new macros do not apply there. No functional change. --- src/backend/commands/explain.c | 8 ++-- src/backend/executor/execRPR.c | 65 ++++++++++++++++---------------- src/backend/optimizer/plan/rpr.c | 4 +- src/include/optimizer/rpr.h | 12 ++++++ 4 files changed, 51 insertions(+), 38 deletions(-) diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c index 53b4e818019..a4b55f79947 100644 --- a/src/backend/commands/explain.c +++ b/src/backend/commands/explain.c @@ -2915,13 +2915,13 @@ static void append_rpr_quantifier(StringInfo buf, RPRPatternElement *elem) { /* Append quantifier if not {1,1} */ - if (elem->min == 0 && elem->max == RPR_QUANTITY_INF) + if (elem->min == 0 && RPRElemIsUnbounded(elem)) appendStringInfoChar(buf, '*'); - else if (elem->min == 1 && elem->max == RPR_QUANTITY_INF) + else if (elem->min == 1 && RPRElemIsUnbounded(elem)) appendStringInfoChar(buf, '+'); else if (elem->min == 0 && elem->max == 1) appendStringInfoChar(buf, '?'); - else if (elem->max == RPR_QUANTITY_INF) + else if (RPRElemIsUnbounded(elem)) appendStringInfo(buf, "{%d,}", elem->min); else if (elem->min == elem->max && elem->min != 1) appendStringInfo(buf, "{%d}", elem->min); @@ -2943,7 +2943,7 @@ append_rpr_quantifier(StringInfo buf, RPRPatternElement *elem) */ if (RPRElemIsAbsorbable(elem)) { - Assert(elem->max == RPR_QUANTITY_INF); + Assert(RPRElemIsUnbounded(elem)); appendStringInfoChar(buf, '#'); } else if (RPRElemIsAbsorbableBranch(elem)) diff --git a/src/backend/executor/execRPR.c b/src/backend/executor/execRPR.c index ce5e79d9f39..16a16221fb8 100644 --- a/src/backend/executor/execRPR.c +++ b/src/backend/executor/execRPR.c @@ -250,9 +250,8 @@ nfa_state_exit_to(WindowAggState *winstate, RPRNFAState *state, int depth, state->isAbsorbable = state->isAbsorbable && RPRElemIsAbsorbableBranch(targetElem); - if (RPRElemIsEnd(targetElem) && - state->counts[targetElem->depth] < RPR_COUNT_INF) - state->counts[targetElem->depth]++; + if (RPRElemIsEnd(targetElem)) + RPRCountIncrement(state->counts[targetElem->depth]); return targetElem; } @@ -884,11 +883,10 @@ nfa_match(WindowAggState *winstate, RPRNFAContext *ctx, RPRVarMatch *varMatched, * Increment count, saturating at RPR_COUNT_INF to avoid int32 * overflow; a saturated count then compares as "unbounded". */ - if (count < RPR_COUNT_INF) - count++; + RPRCountIncrement(count); /* Max constraint should not be exceeded */ - Assert(elem->max == RPR_QUANTITY_INF || count <= elem->max); + Assert(RPRElemWithinMax(elem, count)); state->counts[depth] = count; @@ -919,10 +917,8 @@ nfa_match(WindowAggState *winstate, RPRNFAContext *ctx, RPRVarMatch *varMatched, int32 endCount = state->counts[endDepth]; /* Increment group count */ - if (endCount < RPR_COUNT_INF) - endCount++; - Assert(endElem->max == RPR_QUANTITY_INF || - endCount <= endElem->max); + RPRCountIncrement(endCount); + Assert(RPRElemWithinMax(endElem, endCount)); state->elemIdx = elem->next; state->counts[endDepth] = endCount; @@ -961,10 +957,8 @@ nfa_match(WindowAggState *winstate, RPRNFAContext *ctx, RPRVarMatch *varMatched, state->counts[endDepth] = 0; /* Increment outer group count */ - if (outerCount < RPR_COUNT_INF) - outerCount++; - Assert(outerEnd->max == RPR_QUANTITY_INF || - outerCount <= outerEnd->max); + RPRCountIncrement(outerCount); + Assert(RPRElemWithinMax(outerEnd, outerCount)); state->elemIdx = endElem->next; state->counts[outerDepth] = outerCount; @@ -1018,9 +1012,8 @@ nfa_route_to_elem(WindowAggState *winstate, RPRNFAContext *ctx, * both read. */ landElem = &winstate->rpPattern->elements[skipState->elemIdx]; - if (RPRElemIsEnd(landElem) && - skipState->counts[landElem->depth] < RPR_COUNT_INF) - skipState->counts[landElem->depth]++; + if (RPRElemIsEnd(landElem)) + RPRCountIncrement(skipState->counts[landElem->depth]); } if (skipState != NULL && RPRElemIsReluctant(targetElem)) @@ -1146,7 +1139,7 @@ nfa_advance_begin(WindowAggState *winstate, RPRNFAContext *ctx, Assert(state->counts[elem->depth] == 0); /* Optional group: create skip path (but don't route yet) */ - if (elem->min == 0) + if (RPRElemCanSkip(elem)) { RPRPatternElement *landElem; @@ -1158,9 +1151,8 @@ nfa_advance_begin(WindowAggState *winstate, RPRNFAContext *ctx, * still counts as an iteration of that END's group. */ landElem = &elements[elem->jump]; - if (RPRElemIsEnd(landElem) && - skipState->counts[landElem->depth] < RPR_COUNT_INF) - skipState->counts[landElem->depth]++; + if (RPRElemIsEnd(landElem)) + RPRCountIncrement(skipState->counts[landElem->depth]); } if (skipState != NULL && RPRElemIsReluctant(elem)) @@ -1224,7 +1216,7 @@ nfa_advance_end(WindowAggState *winstate, RPRNFAContext *ctx, int depth = elem->depth; int32 count = state->counts[depth]; - if (count < elem->min) + if (!RPRElemCanExit(elem, count)) { RPRPatternElement *jumpElem; RPRNFAState *ffState = NULL; @@ -1307,7 +1299,7 @@ nfa_advance_end(WindowAggState *winstate, RPRNFAContext *ctx, currentPos); } } - else if (elem->max != RPR_QUANTITY_INF && count >= elem->max) + else if (!RPRElemCanLoop(elem, count)) { /* Must exit: reached max iterations. */ RPRPatternElement *nextElem; @@ -1389,17 +1381,14 @@ nfa_advance_var(WindowAggState *winstate, RPRNFAContext *ctx, { int depth = elem->depth; int32 count = state->counts[depth]; - bool canLoop = (elem->max == RPR_QUANTITY_INF || count < elem->max); - bool canExit = (count >= elem->min); - /* min <= max, so !canExit (count < min) implies canLoop (count < max) */ - Assert(canLoop || canExit); + Assert(RPRElemCanLoop(elem, count) || RPRElemCanExit(elem, count)); /* elem->next must be a valid index for any reachable VAR */ Assert(elem->next >= 0 && elem->next < winstate->rpPattern->numElements); - if (canLoop && canExit) + if (RPRElemCanLoop(elem, count) && RPRElemCanExit(elem, count)) { /* * Both loop and exit possible. Greedy: loop first (prefer longer @@ -1450,14 +1439,26 @@ nfa_advance_var(WindowAggState *winstate, RPRNFAContext *ctx, currentPos); } } - else if (canLoop) + else if (!RPRElemCanExit(elem, count)) { - /* Loop only: keep state as-is */ + /* + * Below the minimum, so exiting is illegal and matching this VAR + * again on the next row is the only legal continuation. This row's + * match already incremented counts[depth] in the match phase, and the + * advance phase only decides where the state goes next, so staying + * parked at the same VAR is expressed by appending the state + * unchanged to the new generation. Dropping it instead would strand + * every quantifier below its minimum: (A B){2} would lose its state + * after the first A B match and never complete. + * + * No clone is needed. With a single continuation, ownership of the + * original simply transfers to the list. + */ nfa_append_state_unique(winstate, ctx, state); } else { - /* Exit only: advance to next element (canExit necessarily true) */ + /* Exit only: advance to next element */ RPRPatternElement *nextElem; nextElem = nfa_state_exit_to(winstate, state, depth, elem->next); @@ -1500,7 +1501,7 @@ nfa_advance_state(WindowAggState *winstate, RPRNFAContext *ctx, Assert(RPRElemIsEnd(hitElem) && RPRElemCanEmptyLoop(hitElem)); - if (state->counts[hitElem->depth] >= hitElem->min) + if (RPRElemCanExit(hitElem, state->counts[hitElem->depth])) { RPRPatternElement *nextElem; diff --git a/src/backend/optimizer/plan/rpr.c b/src/backend/optimizer/plan/rpr.c index 9aa23cb91fc..338e81ddf5a 100644 --- a/src/backend/optimizer/plan/rpr.c +++ b/src/backend/optimizer/plan/rpr.c @@ -1846,7 +1846,7 @@ isUnboundedStart(RPRPattern *pattern, RPRElemIdx idx) RPRPatternElement *e; /* Case 1: Simple unbounded VAR at start (greedy only) */ - if (RPRElemIsVar(elem) && elem->max == RPR_QUANTITY_INF && + if (RPRElemIsVar(elem) && RPRElemIsUnbounded(elem) && !RPRElemIsReluctant(elem)) { /* Set both flags on first element */ @@ -1874,7 +1874,7 @@ isUnboundedStart(RPRPattern *pattern, RPRElemIdx idx) /* END must be unbounded greedy */ if (e->depth == startDepth - 1 && - RPRElemIsEnd(e) && e->max == RPR_QUANTITY_INF && + RPRElemIsEnd(e) && RPRElemIsUnbounded(e) && !RPRElemIsReluctant(e)) { Assert(e->jump == idx); /* END points back to first child */ diff --git a/src/include/optimizer/rpr.h b/src/include/optimizer/rpr.h index 682ed75b48a..0c1e0898397 100644 --- a/src/include/optimizer/rpr.h +++ b/src/include/optimizer/rpr.h @@ -79,6 +79,18 @@ #define RPRElemIsSep(e) ((e)->varId == RPR_VARID_SEP) #define RPRElemIsFin(e) ((e)->varId == RPR_VARID_FIN) #define RPRElemCanSkip(e) ((e)->min == 0) +#define RPRElemIsUnbounded(e) ((e)->max == RPR_QUANTITY_INF) +/* Quantifier tests; a saturated count compares as unbounded */ +#define RPRElemCanLoop(e, count) \ + (RPRElemIsUnbounded(e) || (count) < (e)->max) +#define RPRElemCanExit(e, count) ((count) >= (e)->min) +/* Whether count has stayed inside the bound, not whether it may grow */ +#define RPRElemWithinMax(e, count) \ + (RPRElemIsUnbounded(e) || (count) <= (e)->max) + +/* Count one more iteration, saturating so that int32 cannot overflow */ +#define RPRCountIncrement(count) \ + do { if ((count) < RPR_COUNT_INF) (count)++; } while (0) extern RPRPattern *buildRPRPattern(RPRPatternNode *pattern, List *defineClause, RPSkipTo rpSkipTo, int frameOptions,