Re: Row pattern recognition

Hi Tatsuo,

Sorry for sending these before your next revision of v45 --
I'll be busy with a project proposal for the next couple of
weeks, so I wanted to share them before that.

Here are the patches on top of v45, incorporating the fixes and
improvements I mentioned earlier, plus additional changes from a
thorough code review. There are quite a few patches since I went
ahead without prior discussion -- please review each one individually
and feel free to drop any that you disagree with.

Here's a summary of each patch (to be applied on top of v45):

  0001: Fix mergeGroupPrefixSuffix() max increment
        When absorbing prefix/suffix, child->max was not incremented,
        causing incorrect quantifier bounds. (reported by Zsolt)

  0002: Fix RPR error codes and GROUPS typo
        Use ERRCODE_WINDOWING_ERROR instead of ERRCODE_SYNTAX_ERROR
        for semantic window-frame violations. Fix "GROUP" typo to
        "GROUPS" in frame validation error message. (reported by Zsolt)

  0003: Add check_stack_depth() and CHECK_FOR_INTERRUPTS()
        Add stack depth protection to recursive pattern optimization
        and interrupt checks to NFA engine loops.

  0004: Fix window_last_value set_mark during RPR
        Restore set_mark=true in window_last_value (normal behavior)
        and add an RPR-specific override inside WinGetFuncArgInFrame.
        This keeps the RPR workaround localized rather than changing
        the caller's semantics unconditionally.

  0005: Fix row_is_in_reduced_frame in WINDOW_SEEK_TAIL
        Pass frameheadpos directly to row_is_in_reduced_frame
        instead of frameheadpos+relpos. Currently only last_value()
        calls this path with relpos=0 so no actual bug, but the
        old expression would be incorrect for negative relpos.
        Also add a bounds check for future callers.

  0006: Clarify ST_NONE intent
        Add comment explaining ST_NONE = 0 is the default for
        non-RPR windows.

  0007: Clarify inverse transition optimization comment
        Document why RPR disables inverse transition: the reduced
        frame changes row by row.

  0008: Reject unused DEFINE variables
        I know you preferred silently ignoring unused DEFINE
        variables [1], and I agree the standard doesn't mandate
        an error. However, if we later add qualified column
        references (e.g. B AS A.price > 100), B's expression
        depends on A being present. If A is not used in PATTERN
        and the planner silently removes it, B's reference to A
        becomes dangling. I worry that silently allowing this now
        could create forward-compatibility problems once qualified
        references are introduced. For that reason, I'm inclined
        to think rejecting them now may be safer than changing
        behavior later, which would be a user-visible compatibility
        break. This is also consistent with Oracle's behavior
        (ORA-62503), as SungJun reported, and it helps catch user
        typos in DEFINE variable names at parse time.

  0009: Clarify RPR documentation in advanced.sgml
        Improve absorption explanation and clarify non-match row
        aggregation behavior with concrete examples.

  0010: Fix typos in RPR comments and parser README

  0011: Clarify excludeLocation and empty quantifier in gram.y
        Add comments explaining the conditional location assignment
        pattern and the empty quantifier rule.

  0012: Clarify RPR_VARID_MAX definition
        Document varId range 0-250 and reserved control element
        values 252+.

  0013: Move local variables to function scope
        In row_is_in_reduced_frame, move declarations out of
        switch case blocks.

  0014: Reset reduced_frame_map pointer in release_partition
        Set reduced_frame_map = NULL and alloc_sz = 0 to prevent
        dangling pointer after partition context reset.

  0015: Remove redundant list manipulation in nfa_add_matched_state
        Simplify doubly-linked list operation that was duplicated
        by the subsequent ExecRPRFreeContext() call.

  experimental: Implement 1-slot PREV/NEXT navigation
        The slot-based PREV/NEXT approach I mentioned earlier.
        This no longer needs attno_map, so the plan cache mutation
        issue is gone. PREV/NEXT now also accepts an optional
        second argument for offset, e.g. PREV(price, 2).

        Note: all existing regression tests pass with this patch,
        but my understanding of TupleSlot internals and JIT is still
        shallow -- the implementation largely copies surrounding
        patterns, so there may be issues I haven't caught yet. I'd
        like to keep this as a separate patch on top of v46 until it
        is trustworthy enough, rather than folding it into the main
        series. I'll continue reviewing and hardening it.

One design decision regarding PREV/NEXT: the SQL standard
requires that the first argument of PREV/NEXT contain at least
one column reference [2] -- e.g. PREV(1) is a syntax error.
Beyond the standard's rationale (no starting row for offsetting),
there is a practical reason: when the target row falls outside
the frame, PREV should return NULL, but a constant expression
like PREV(42) would still evaluate to 42 since it never reads
from the slot. I think it's correct to follow the standard and
reject PREV/NEXT without a column reference. What do you think?

[1]
https://www.postgresql.org/message-id/20260305.142049.1864331791480656300.ishii%40postgresql.org
[2] ISO/IEC 19075-5, Subclause 5.6.2

Best regards,
Henson