Re: Row pattern recognition

From: Henson Choi <assam258(at)gmail(dot)com>
To: Tatsuo Ishii <ishii(at)postgresql(dot)org>
Cc: zsolt(dot)parragi(at)percona(dot)com, sjjang112233(at)gmail(dot)com, vik(at)postgresfriends(dot)org, er(at)xs4all(dot)nl, jacob(dot)champion(at)enterprisedb(dot)com, david(dot)g(dot)johnston(at)gmail(dot)com, peter(at)eisentraut(dot)org, li(dot)evan(dot)chao(at)gmail(dot)com, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Row pattern recognition
Date: 2026-05-01 17:11:00
Message-ID: CAAAe_zBdwAUDNs_WFdLkFF=ewhkDkv-AqizVEVzhsfremGFb4w@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

Attachment Content-Type Size
nocfbot-0001-Remove-unused-regex-include.txt text/plain 767 bytes
nocfbot-0002-CHECK_FOR_INTERRUPTS-nfa_add_state_unique.txt text/plain 832 bytes
nocfbot-0003-CHECK_FOR_INTERRUPTS-nfa_try_absorb_context.txt text/plain 866 bytes
nocfbot-0004-Fix-defineClause-TargetEntry-copy.txt text/plain 1.4 KB
nocfbot-0005-Fix-mark-handling-last_value-RPR.txt text/plain 1.9 KB
nocfbot-0006-Fix-DEFINE-expression-handling-RPR-window-planning.txt text/plain 8.2 KB
nocfbot-0007-Add-RPR-planner-integration-tests.txt text/plain 89.6 KB
nocfbot-0008-Replace-reduced-frame-map-with-single-match-result.txt text/plain 21.1 KB
nocfbot-0009-Add-fixed-length-group-absorption-for-RPR.txt text/plain 55.5 KB
nocfbot-0010-Rename-rpr_explain-test-views-to-descriptive-names.txt text/plain 127.8 KB
nocfbot-0011-Fix-quote_identifier-deparse.txt text/plain 7.3 KB
nocfbot-0012-Fix-execRPR-Makefile-ordering.txt text/plain 729 bytes
nocfbot-0013-Remove-unused-force_colno.txt text/plain 2.7 KB
nocfbot-0014-CHECK_FOR_INTERRUPTS-cleanup-finalize.txt text/plain 1.1 KB
nocfbot-0015-Narrow-variable-scope-DEFINE-loop.txt text/plain 1.3 KB
nocfbot-0016-Normalize-flag-macros-to-bool.txt text/plain 1.4 KB
nocfbot-0017-Implement-1-slot-PREV-NEXT-navigation-for-RPR.txt text/plain 85.4 KB
nocfbot-0018-JIT-support-for-PREV-NEXT.txt text/plain 7.0 KB
nocfbot-0019-Add-tuplestore-trim-optimization-for-RPR-PREV.txt text/plain 12.3 KB
nocfbot-0020-Update-RPR-code-comments-for-1-slot-navigation.txt text/plain 5.5 KB
nocfbot-0021-Enable-JIT-PREV-NEXT-tests.txt text/plain 1.8 KB
nocfbot-0022-Add-2-arg-PREV-NEXT-host-variable-test.txt text/plain 4.8 KB
nocfbot-0023-Nav-Mark-Lookback-EXPLAIN.txt text/plain 107.6 KB
nocfbot-0024-Implement-FIRST-LAST-navigation-for-RPR.txt text/plain 165.2 KB
nocfbot-0025-Guard-int64-overflow-bounded-frame.txt text/plain 2.4 KB
nocfbot-0026-Fix-error-message-style.txt text/plain 7.4 KB
nocfbot-0027-Fix-comment-typos-grammar.txt text/plain 5.3 KB
nocfbot-0028-Fix-documentation-synopsis-grammar.txt text/plain 9.5 KB
nocfbot-0029-Fix-nav_slot-pass-by-ref-dangling-pointer.txt text/plain 10.0 KB
nocfbot-0030-Add-inline-comments-design-notes.txt text/plain 11.2 KB
nocfbot-0031-Remove-unused-include-fix-header-ordering.txt text/plain 1.9 KB
nocfbot-0032-Fix-A1-frame-optimization-bypass-test.txt text/plain 5.4 KB
nocfbot-0033-Fix-imperative-voice-check_rpr_nav_expr-comment.txt text/plain 1.3 KB
nocfbot-0034-Fix-passive-voice-RPR-error-messages.txt text/plain 22.0 KB
nocfbot-0035-Use-Max-Min-macros-RPR-extremum.txt text/plain 5.0 KB
nocfbot-0036-Reword-RPR-navigation-function-synopses.txt text/plain 3.4 KB
nocfbot-0037-Replace-non-ASCII-characters-RPR-code-tests.txt text/plain 29.0 KB
nocfbot-0038-Move-RPR-design-notes-to-README.rpr.txt text/plain 129.0 KB
nocfbot-0039-Apply-pgindent-RPR-comment-blocks.txt text/plain 5.3 KB
nocfbot-0040-Reword-RPR-navigation-descriptions-pg_proc.dat.txt text/plain 3.6 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andrew Dunstan 2026-05-01 17:26:51 Re: Add errdetail() with PID and UID about source of termination signal
Previous Message Andres Freund 2026-05-01 16:58:41 Re: Why clearing the VM doesn't require registering vm buffer in wal record