From 2858fb7dd188ead146774d35e5e648c453532949 Mon Sep 17 00:00:00 2001 From: jian he Date: Sun, 16 Aug 2026 14:23:15 +0900 Subject: [PATCH] Assert the two RPR absorption invariants where each is relied on Both places that settle a state's isAbsorbable were deciding what they could have asserted. nfa_states_equal() compares elemIdx and the live counts, and isAbsorbable follows from those two: it holds only while every element on the state's path carried ABSORBABLE_BRANCH, and computeAbsorbability() marks a leftmost prefix, so a path that once left the region can re-enter only through a group's END, whose arrival increments the count at a depth shallower than the current element -- one the memcmp already covers -- while a state that never left is still zero there. The invariant is load-bearing rather than decorative: nfa_add_state_unique() keeps the existing state of a merged pair, so were it to fail, a merge could leave allStatesAbsorbable set over a context whose future is not absorbable, and nfa_try_absorb_context() would free a context holding a match no older one reproduces. Extending absorption past a leading unbounded segment, noted as future work in rpr.c, is the change that would put the leftmost-prefix premise in question. Assert it. ExecRPRStartContext() branched on element 0's ABSORBABLE_BRANCH to choose the initial state's flag, and cleared the context's two absorption flags along the false arm. computeAbsorbability() starts its walk at element 0 with a fresh accumulator, and each of its three arms marks that element exactly when it reports the pattern absorbable, so the branch asks what pattern->isAbsorbable already answers. The identity holds at index 0 and not in general -- the arms that mark on the accumulator read a value an earlier sibling can have set -- which is reason enough to assert it rather than leave it implied. The two cleared flags were already dead: nfa_context_make() takes both from that same field a few lines earlier. No expected output moves, in either build. --- src/backend/executor/execRPR.c | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/src/backend/executor/execRPR.c b/src/backend/executor/execRPR.c index 68664568556..5ac4ab21f2f 100644 --- a/src/backend/executor/execRPR.c +++ b/src/backend/executor/execRPR.c @@ -288,6 +288,9 @@ nfa_states_equal(WindowAggState *winstate, RPRNFAState *s1, RPRNFAState *s2) if (memcmp(s1->counts, s2->counts, sizeof(int32) * compareDepth) != 0) return false; + /* isAbsorbable follows from the element and the counts compared above */ + Assert(s1->isAbsorbable == s2->isAbsorbable); + return true; } @@ -1683,24 +1686,20 @@ ExecRPRStartContext(WindowAggState *winstate, int64 startPos) { RPRNFAContext *ctx; RPRPattern *pattern = winstate->rpPattern; - RPRPatternElement *elem; ctx = nfa_context_make(winstate); ctx->matchStartRow = startPos; ctx->states = nfa_state_make(winstate); /* initial state at elem 0 */ - elem = &pattern->elements[0]; - - if (RPRElemIsAbsorbableBranch(elem)) - { - ctx->states->isAbsorbable = true; - } - else - { - ctx->hasAbsorbableState = false; - ctx->allStatesAbsorbable = false; - ctx->states->isAbsorbable = false; - } + /* + * The only state so far sits on element 0, and computeAbsorbability() + * marks that element ABSORBABLE_BRANCH exactly when it calls the pattern + * absorbable, so the pattern's flag answers for the state -- as it + * already did for the context flags nfa_context_make() set. + */ + Assert(RPRElemIsAbsorbableBranch(&pattern->elements[0]) == + pattern->isAbsorbable); + ctx->states->isAbsorbable = pattern->isAbsorbable; /* * Add to tail of active context list (doubly-linked, oldest-first).