From 57989dcb91d0c0e37d07c4d4b79004c45b2815aa Mon Sep 17 00:00:00 2001 From: Henson Choi Date: Mon, 1 Jun 2026 13:45:48 +0900 Subject: [PATCH 34/68] Correct stale RPR comments and document a defensive window check Static analysis surfaced several comments that no longer match the code: - plannodes.h: rewrite the RPRPattern.isAbsorbable comment to match the implemented cases (isUnboundedStart / computeAbsorbability) and the normalization that enables absorption; the old text wrongly called the prefix/suffix merge unimplemented - README.rpr: an unmatched row's reduced frame is empty, not "the row itself"; the varId range is 0-250, not 0-251 - advanced.sgml: unmatched rows show NULL window functions and initial-value aggregates (count() = 0), not all-NULL - planner.c: note that the RPR fields in optimize_window_clauses' duplicate check are reached but defensive -- RPR clauses are separated by their frame options first -- kept for parity with transformWindowFuncCall --- doc/src/sgml/advanced.sgml | 5 +++-- src/backend/executor/README.rpr | 6 +++--- src/backend/optimizer/plan/planner.c | 5 ++++- src/include/nodes/plannodes.h | 21 ++++++++++++++------- 4 files changed, 24 insertions(+), 13 deletions(-) diff --git a/doc/src/sgml/advanced.sgml b/doc/src/sgml/advanced.sgml index 11c2416df51..1410a443609 100644 --- a/doc/src/sgml/advanced.sgml +++ b/doc/src/sgml/advanced.sgml @@ -600,8 +600,9 @@ DEFINE subsequent rows all window functions are shown as NULL. Aggregates on non-starting rows return their initial value: for example, count() returns 0 and sum() - returns NULL. For rows that do not match the PATTERN, columns are shown - as NULL too. Example of a SELECT using + returns NULL. For rows that do not match the PATTERN, window functions + are likewise shown as NULL and aggregates return their initial value. + Example of a SELECT using the DEFINE and PATTERN clause is as follows. diff --git a/src/backend/executor/README.rpr b/src/backend/executor/README.rpr index 449bf051153..08418588114 100644 --- a/src/backend/executor/README.rpr +++ b/src/backend/executor/README.rpr @@ -245,7 +245,7 @@ RPRPatternElement struct (16 bytes): Field Size Description --------------------------------------------------------- - varId 1B Variable ID (0-251) or control code (252-255) + varId 1B Variable ID (0-250) or control code (252-255) depth 1B Group nesting depth flags 1B Bit flags (see below) reserved 1B Padding @@ -1303,11 +1303,11 @@ XI-4. Execution Trace XI-5. Final Result - Row 0: unmatched -> frame = the row itself + Row 0: unmatched -> reduced frame empty (window funcs NULL, count() 0) Row 1: match head -> frame = rows 1 through 3 Row 2: inside match -> skipped Row 3: inside match -> skipped - Row 4: unmatched -> frame = the row itself + Row 4: unmatched -> reduced frame empty (window funcs NULL, count() 0) Chapter XII Summary of Key Design Decisions ============================================================================ diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c index c6fc868cdca..f43cc0edb37 100644 --- a/src/backend/optimizer/plan/planner.c +++ b/src/backend/optimizer/plan/planner.c @@ -6254,7 +6254,10 @@ optimize_window_clauses(PlannerInfo *root, WindowFuncLists *wflists) /* * Perform the same duplicate check that is done in - * transformWindowFuncCall. + * transformWindowFuncCall. wc is never an RPR clause here + * (those are skipped above), and an RPR existing_wc differs + * in its frame options anyway, so the RPR-related comparisons + * are a defensive backstop for parity. */ if (equal(wc->partitionClause, existing_wc->partitionClause) && equal(wc->orderClause, existing_wc->orderClause) && diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h index fd4bdf2cb31..d45d93d79a2 100644 --- a/src/include/nodes/plannodes.h +++ b/src/include/nodes/plannodes.h @@ -1296,15 +1296,22 @@ typedef struct RPRPattern * Context absorption optimization. * * Absorption is only safe when later matches are guaranteed to be - * suffixes of earlier matches. This requires simple pattern structure: + * suffixes of earlier matches, which requires the pattern to start with + * an unbounded greedy element. Phase-1 normalization (consecutive + * variable / group / ALT merging and prefix/suffix merging) rewrites the + * pattern toward that form first -- so e.g. A B (A B)+ is merged to + * (A B){2,} and then judged absorbable. * - * Case 1: No ALT, single unbounded element (A+, (A B)+) - * Case 2: Top-level ALT with each branch being single unbounded (A+ | B+) + * computeAbsorbability() marks the absorbable cases (see isUnboundedStart): + * - simple unbounded VAR at the start: A+ B C + * - unbounded GROUP with fixed-length children: (A B)+, (A B{2})+ + * - top-level ALT with independently absorbable branches: A+ | B+ + * (handled in computeAbsorbabilityRecursive) * - * Complex patterns like A B (A B)+ could theoretically be transformed to - * (A B){2,} for absorption, but this changes lexical order and is not - * implemented. Similarly, (A|B)+ cannot be absorbed because different - * start positions produce different match contents (not suffix relation). + * Not absorbable: an unbounded element not at the start (A B+), a + * reluctant quantifier (A+?), or an ALT inside a group ((A|B)+) -- there + * different start positions yield different match contents, so later + * matches are not suffixes of earlier ones. */ bool isAbsorbable; /* true if pattern supports context absorption */ } RPRPattern; -- 2.50.1 (Apple Git-155)