Thread

  1. Re: Row pattern recognition

    Henson Choi <assam258@gmail.com> — 2026-05-01T17:11:00Z

    Hi Tatsuo,
    
    
    Attached are 40 incremental patches on top of v46, with all
    review-driven follow-ups folded in.  The 0001-0031 portion is
    unchanged from the previous send (filenames preserved).  The new
    patches 0032-0040 are the delta + follow-up patches we agreed to
    deliver on top of 0031, plus two cosmetic patches that surfaced
    during the sweep.
    
    As you suggested, I have not refreshed 0001-0031; the new work
    is layered on top so that the existing review state on those
    patches stays valid.
    
    Numbering and subjects of 0001-0031 are unchanged.  The new
    patches are:
    
    
      - 0032 Fix A1 frame optimization bypass test in
        rpr_integration
    
        Per your A1 review.  Switches the baseline to row_number()
        with an explicit ROWS frame so that the bypass guard is
        actually exercised.  Block comment reworded in A2/A3 style.
    
    
      - 0033 Fix imperative voice in check_rpr_nav_expr header
        comment
    
        Per your typo note: "Checks for" -> "Check for".
    
    
      - 0034 Fix passive voice in RPR error messages
    
        Per your style note.  Sweep of RPR-touched code paths
        (parse_rpr.c, parse_func.c, windowfuncs.c) for the
        "is not permitted when ... is used" / "can only be used in"
        patterns, rewritten to "cannot use ... with" /
        "cannot use ... outside".
    
    
      - 0035 Use Max()/Min() macros for RPR extremum accumulation
    
        Per your Max() macro suggestion.  Sweep of all hand-rolled
        "if (x > y) y = x;" sites in createplan.c, nodeWindowAgg.c,
        and execRPR.c.  No double-evaluation hazards on the call
        sites.
    
    
      - 0036 Reword RPR navigation function synopses
    
        Per your sgml note.  "Returns the column value at the row"
        -> "Returns value evaluated at the row" (the first argument
        can be an arbitrary expression).
    
    
      - 0037 Replace non-ASCII characters in RPR code and tests
    
        Per your non-ASCII note.  Sweep of RPR-touched files;
        mostly comments and rpr*.out.
    
    
      - 0038 Move RPR design notes from execRPR.c to README.rpr
    
        Per the README.rpr split we discussed.  Extracts the
        design block at the head of execRPR.c into
        src/backend/executor/README.rpr (plain text, matching the
        executor/README convention).  No code changes.
    
    
      - 0039 Apply pgindent canonicalization to RPR comment blocks
    
        Cosmetic.  pgindent sweep across RPR-touched files; labeled
        blocks wrapped in /*---------- ... *---------- to preserve
        hanging indents.  No code changes.
    
    
      - 0040 Reword RPR navigation function descriptions in
        pg_proc.dat
    
        Cosmetic.  Aligns the eight RPR navigation descr fields
        with the surrounding "fetch the ... row value" pattern used
        by lag / lead / first_value / last_value / nth_value.
    
    
    On the items deferred to post-v47:
    
    
      - DEFINE volatile prohibition: prepared as a self-contained
        follow-up so it can be reviewed independently of v47.
    
      - 0009 nfaVisitedElems representation: per your decision,
        kept as-is.  Three points behind that:
    
          (1) Size concern.  visited is 1 bit per element, a flat
              NFA element is 16 bytes -- a fixed 128:1 ratio
              (~0.78%).  The bitmap is dwarfed by the NFA it
              tracks (worst case ~4 KiB next to ~512 KiB), so it
              is not a memory hot spot.
    
          (2) Bitmapset misfit.  Its invariants ("no trailing zero
              word", "empty set = NULL") map the per-advance reset
              onto pfree + palloc-on-next-add, which defeats the
              in-place / fixed-capacity reuse pattern this code
              relies on.
    
          (3) Future variant.  A high-water mark approach (track
              the min/max touched word index, memset only that
              span on reset) preserves the in-place pattern and
              shrinks reset cost from O(numElements/64) to
              O(span_words).  Held as a future optimization
              candidate.
    
      - 0007 / B7 (Recursive CTE XXX): tracked as a discussion item
        awaiting community input on the standard interpretation.
    
    
    Regards,
    Henson