Re: Row pattern recognition

Hi all,

Resolved since the last post:

D1. Single-row frame conformance, Subclause 6.10.2 -- Tatsuo's call [6]
was to reject. Both ROWS BETWEEN CURRENT ROW AND CURRENT ROW and
AND 0 FOLLOWING are now rejected (nocfbot-0025), which in turn
unblocks the held ExecRPRProcessRow change (nocfbot-0026).

Open decisions (repeated with context at the end):

D2. The RPRContext consolidation -- Tatsuo's call as co-author;
non-blocking either way [4].
D3. The AST "absorption" rename -- Tatsuo's call [2].

[1] Jian's review, round 1 (2026-05-26):

https://postgr.es/m/CACJufxH-DZePhbdJM=8nNYceQiSbbXXLTw54iLhxiynQ+4hbBA@mail.gmail.com
[2] my round-1 reply -- AST "absorption" rename deferred to Tatsuo (D3,
2026-05-27):

https://postgr.es/m/CAAAe_zDephfiDA_A3FN0hCymJRogEr=Rt3QoCTf4qMYDLk+xNA@mail.gmail.com
[3] Jian's review, round 2 (2026-05-28):

https://postgr.es/m/CACJufxGX17thWuEOq1tM5xbRHz2HXm1asooZC3GV25MYGmYqLQ@mail.gmail.com
[4] my round-2 reply -- RPRContext deferred to Tatsuo (D2, 2026-05-29):

https://postgr.es/m/CAAAe_zAH0MvP0TBmW3PTLeHjEpiyBz0473zJRM9pwLpseefMNw@mail.gmail.com
[5] single-row frame conformance question (D1, 2026-05-29):

https://postgr.es/m/CAAAe_zCbSU=dd-4qTL2QaBQwQ-cf51N_851a9Y5rOoz0wj0aXw@mail.gmail.com
[6] Tatsuo's reply -- reject single-row frames (D1 resolved, 2026-05-30):

https://postgr.es/m/20260530.114737.1416684464524168377.ishii@postgresql.org
[7] Jian's review, round 3 (2026-05-30):

https://postgr.es/m/CACJufxEsaU8GQ4yeXTWhAO8VjbrZTh5CpvUqz=4a3T0Cwz44pA@mail.gmail.com

Attached: the v47 feature series (v47-0001..0009) rebased onto current
master, plus the incremental patch series carried on top of it.

Base:

9a41b34a287 2026-05-26 doc: add comma to UPDATE docs, for consistency

Unchanged -- rebase only (already posted; only rebased, no content change).
Titles for reference:

nocfbot-0001 Add DEFINE non-volatile baseline to rpr_integration B9
nocfbot-0002 Unify RPR DEFINE walkers and reject volatile callees
nocfbot-0003 Cover RPR empty-match path with EXPLAIN tests; fix stale
XXX comments
nocfbot-0004 Reclassify DEFINE qualifier check and reword diagnostic to
"expression"
nocfbot-0005 Sync stale comments on DEFINE/PATTERN handling
nocfbot-0006 Add trailing commas to RPR enum definitions
nocfbot-0007 Remove optional outer parentheses from ereport() calls in
RPR files
nocfbot-0008 Add high-water mark tracking to NFA visited bitmap reset
nocfbot-0009 Document DEFINE subquery rejection as intentional
over-rejection
nocfbot-0010 Remove duplicate #include in nodeWindowAgg.c
nocfbot-0011 Normalize SQL/RPR standard references
nocfbot-0012 Add rpr_integration B7 cases for RPR in recursive query
nocfbot-0013 Reject row pattern recognition in recursive queries
nocfbot-0014 Enhance README.rpr per Tatsuo Ishii's review
nocfbot-0015 Round out README.rpr WindowAggState field coverage
nocfbot-0016 Add raw_expression_tree_walker coverage for RPR raw nodes

(nocfbot-0016 was sent earlier as 0015; renumbered here so the review
series runs contiguously from 0017.)

New incremental patches -- nocfbot-0017 onward. These apply Jian He's
review (rounds 1 [1] and 2 [3]) and settle D1. nocfbot-0017..0024 and
0026 are comment / doc / test plus a signature change and a couple of
Assert additions -- no behavior change. nocfbot-0025 is the one
user-visible change: it rejects the single-row frame per D1 [6].

nocfbot-0017 Enhance README.rpr
Chapter VIII absorption intro + a worked PATTERN (A+) trace;
"Depth-First Search" spelled out at first use; the stale
nfa_advance(initialAdvance=...) reference replaced.

nocfbot-0018 Clarify execRPR.c comments and tighten an Assert
Document the NFA invariants (compareDepth slot arithmetic, the
asymmetric visited-marking scheme, the greedy/non-nullable BEGIN
label, the ALT depth break, the state->next reset boundary),
reframe ExecRPRFinalizeAllContexts as the partition-end policy
holder, and add a defensive Assert in nfa_advance_var.

nocfbot-0019 nfa_add_state_unique: bool return -> void
The return value was unused.

nocfbot-0020 Reluctant bounded mid-band test (rpr_nfa)
A{3,5}? B, which drives the VAR-level count in nfa_advance_var
through 3..5.

nocfbot-0021 Define RPR absorption terminology in README.rpr
Terminology block for the "match_start dep." column (none /
direct / boundary check), the "boundary chk" typo fix, the
count-dominance definition, and the match_start_dependent rename.

nocfbot-0022 Document the get_reduced_frame_status cascade invariant
The branches form an order-dependent early-return cascade; the
running invariant is spelled out so the order reads as intentional.

nocfbot-0023 Explain the completed-head-context branch in
update_reduced_frame
Why a head context can already be complete under SKIP TO NEXT ROW.

nocfbot-0024 Tighten the frame-boundary check from >= to ==
The > case is unreachable by the loop invariant; the defense moves
into an Assert(currentPos <= ctxFrameEnd), and the comment changes
from "exceeded" to "reached".

nocfbot-0025 Reject single-row window frame in row pattern recognition
Per D1 [6], the frame end must be UNBOUNDED FOLLOWING or a positive
offset FOLLOWING. CURRENT ROW is rejected in transformRPR() at parse
time; a zero offset -- which need not be a constant -- in
calculate_frame_offsets() at run time. The two single-row rpr_base
tests become error cases, with a bind-parameter error test added.

nocfbot-0026 Remove the redundant zero check on the RPR frame ending
offset
With the single-row frame now rejected, a limited frame always
carries a real offset, so the "endOffsetValue != 0" guard -- which
compared a Datum directly to zero -- is dropped, leaving a plain
DatumGetInt64(). This is the offset half of Jian's ExecRPRProcessRow
cleanup; the structural half (dropping the hasLimitedFrame
parameter) I've left as-is -- it is loop-invariant and computed once
outside the per-row loop, so moving it into ExecRPRProcessRow would
just repeat the work each row.

Coverage -- my decision summaries [2], [4], with the patch each became:

Round 1 [2]
Short-circuit optimization Separate series -> separate series
Absorption README narrative Accept -> nocfbot-0017
AST-level "absorption" rename Pending Tatsuo -> D3
DFS expansion Accept -> nocfbot-0017
initialAdvance README mismatch Accept -> nocfbot-0017
Defensive Assert in advance_var Accept -> nocfbot-0018
Finalize unnecessary? Keep -> nocfbot-0018
Greedy comment label Accept -> nocfbot-0018
state->next reset Decline -> nocfbot-0018
count >= 3 test coverage Accept -> nocfbot-0020
visited marking purpose Accept -> nocfbot-0018
compareDepth comment Accept -> nocfbot-0018
Unused bool return Accept -> nocfbot-0019
ALT depth invariant Assert Decline -> nocfbot-0018

Round 2 [4]
RPRContext consolidation Tatsuo's call -> D2
get_reduced_frame_status order Keep -> nocfbot-0022
README terminology + typo Accept -> nocfbot-0021
ExecRPRProcessRow refactor Datum fix only -> nocfbot-0026
single-row frame (6.10.2, D1) Reject (Tatsuo) -> nocfbot-0025
currentPos >= -> == Accept -> nocfbot-0024
states == NULL branch coverage Already reached -> nocfbot-0023

Remaining work and decisions

Decisions (need your input):
D2 RPRContext consolidation -- Tatsuo.
D3 AST "absorption" rename -- Tatsuo.

Held pending a decision:
- the RPRContext consolidation itself -- done if D2 says go.
- the AST "absorption" rename itself -- done if D3 says go.

(D1 settled: the parse-time frame-end check, the offset-0 test as an
error case, and the Datum-comparison fix are now in nocfbot-0025/0026.)

From Jian's round-3 review [7] (into the next revision):
- four comment fixes: the line-number test anchors, the "at the END"
and uppercase-END wording, and the nfa_advance_var count premise.
- the make/clone rename of the NFA helpers (one of three attached
refactors); the other two -- dropping nfaStateSize and the elem
parameter -- I'd keep, both on hot-path grounds.

From our in-house review (separate follow-ups):
- a quantifier-normalization correctness fix (nested unbounded
quantifiers such as (A{2,})* are currently mis-normalized)
- a per-tuple memory-context fix in DEFINE evaluation (still verifying)
- smaller correctness/conformance fixes (an overflow guard, a few
missing parse-time checks, EXPLAIN output details)
- documentation gaps (comments, README, SGML)
- added regression tests (round-trip deparse, edge-case offsets)

Before the v48 fold (from Jian's off-list comments):
- INT_MAX -> PG_INT32_MAX (the unbounded-quantifier sentinel; ~24 sites)
- foreach + lfirst() -> foreach_node (~33 sites)
- foreach_current_index, dropping the redundant break (3 sites)

Work, no decision needed:
- short-circuit (lazy eval) -- stop evaluating a DEFINE predicate
once its outcome is fixed. A separate series.
It turns on a standard-interpretation point -- whether skipping is
sound when a dropped subexpression has side effects.

Thanks again to Jian for the careful reading.

Henson