Thread
-
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