| From: | Henson Choi <assam258(at)gmail(dot)com> |
|---|---|
| To: | jian he <jian(dot)universality(at)gmail(dot)com>, Tatsuo Ishii <ishii(at)postgresql(dot)org> |
| Cc: | zsolt(dot)parragi(at)percona(dot)com, sjjang112233(at)gmail(dot)com, 신성준 <shinsj4653(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-08-30 02:04:06 |
| Message-ID: | CAAAe_zD8ZNHVxFadQDY16YZeLrmV_u5R2wTzQUPmdWUhPwWtyg@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi hackers,
This is the fourth of the postings that follow the increment, and
the last of them. It covers 2022 through 2026 and changes nothing
in them.
What holds them together is that none of them is about what a match
returns. They are about what the feature leaves outside itself --
the query id it reports, the text it deparses to, and what the tests
and the documentation say it does. 2026 stands in all three at
once, which is why it comes last.
2022 Correct stale RPR test comments and fill gaps in the RPR
documentation
No code changes. The only SQL statement that moves is one that is
deleted, a GROUP BY + RPR case whose recorded output is a syntax
error, so no change to RPR could ever have made it fail.
The test comments the audit found have the same shape throughout:
prose the recorded output already contradicts.
- a DEFINE variable that PATTERN does not use was said to be
ignored, where the query is rejected;
- a match was attributed to ONE ROW PER MATCH, which this feature
does not implement;
- the reduced frame of the last_value test was said to hold only
NULLs, and the reluctant-quantifier comparison named the wrong
column as the outlier;
- several EXPLAIN tests claimed absorption, state merging or a
skip count the recorded plan does not show.
Names that do not exist go with them, and so does the development
history -- a reviewer credited by name, a pointer to a future patch,
and three comments narrating bugs that were fixed.
The documentation gains what it never said:
- the four sub-clauses and the order they must be written in;
- what a DEFINE expression may contain, and that DEFINE itself is
not optional;
- what an unmatched row shows for a function that reads the frame
against one that does not;
- that the frame must be written in ROWS mode;
- the 254-level depth limit 2020 arrives at, and the PERMUTE
rejection 2019 adds.
2023 Jumble RPR DEFINE variable names
A window's DEFINE clause is stored as a list of TargetEntry whose
resname is the pattern variable being defined. resname carries
query_jumble_ignore because a column alias must not affect the query
id, and that is right for an ordinary target list. It is wrong here
only because the name is not an alias but the variable the condition
defines, so
DEFINE A AS p > 50, B AS p < 50
DEFINE B AS p > 50, A AS p < 50
are two different queries that collapsed onto one query id.
2024 Section the trailing RPR integration tests and gather the
planner guards
A test relocation; no production code is touched. What had piled up
at the end of the integration file takes the numbered headers the
rest of it uses -- B11 junk targetlist pruning, B12 correlated
navigation offsets, B13 DEFINE-only parameter caching, B14 multiple
window definitions -- and B10, which the index listed without saying
what it covers, is named.
The hundred-line block of planner guards that rpr_explain.sql
carried asserts nothing about the NFA statistics that file exists to
check, and was not in its index either. A1 and A2 in the
integration file already assert the frame-optimization and
run-condition properties, so the two redundant pairs go and the
function lists they carried move into the A1 and A2 comments. That
is where the 377 lines removed against 221 added comes from.
rpr_ev_opt_mixed is the one case A3 does not cover, so it moves here
as a view and is left undropped on purpose: it is the only view in
the tree that serializes an RPR window together with a non-RPR one,
which is what pg_upgrade and pg_dump need in order to exercise that
round trip. The file header says so, since the file otherwise drops
what it creates.
2025 Remove stray spaces and column qualifiers from deparsed RPR
clauses
Two defects in ruleutils.c. Here is what pg_get_viewdef() printed,
with each stray space written as a dot:
WINDOW w AS (... ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING.
AFTER MATCH SKIP PAST LAST ROW
INITIAL
PATTERN (start up+)
DEFINE
start AS true,
up AS (val > PREV(val)).);
- get_rule_windowspec() printed AFTER MATCH, INITIAL, PATTERN and
DEFINE as four blocks, each gated on needspace and each opening
a newline of its own, so the separator space landed at the end
of the frame clause's line; DEFINE left one more before the
closing parenthesis. The four only ever appear together --
PATTERN is mandatory in the row pattern common syntax, and <row
pattern initial or seek> yields INITIAL for both spellings the
grammar accepts -- so they fold into the rpPattern branch,
needspace goes, and AFTER MATCH becomes an if/else with an
Assert. The closing parenthesis stays outside the branch, so a
non-RPR window still round-trips.
- get_rule_define() printed a DEFINE column with whatever
qualification the Var carried, so a view over two relations
deparsed to "a AS ttt.val < 0". The qualifier slot in a DEFINE
expression is for pattern variables, and the parser rejects
every qualified spelling, which leaves the bare name as the only
re-parseable one. varprefix is false for the clause now, and
restored afterwards.
The expected output moves by some 1700 lines, nearly all of it the
re-recording that losing one trailing space forces on every deparsed
window.
What is left over is an ambiguity this patch does not close, and the
expected output records it rather than hiding it. A view over more
than one relation can still deparse to a DEFINE column that has
become ambiguous, the grammar admitting no qualifier to disambiguate
it with. In rpr_base.sql it is sv3, over sa JOIN sb where both
sides have a price column, and what the expected output holds under
the comment "the deparsed text above no longer re-parses" is
ERROR: column reference "price" is ambiguous
That view is unrestorable from the moment it is created, so the
limitation is not confined to columns that become ambiguous later.
The serialization tests had no view over more than one relation at
all, so the varprefix change had no coverage among them; one is
added, and the ambiguity cases go into the same section.
This is the failure Tatsuo reported on 08-14 and fixed himself on
08-21. rule-fix.txt and this patch fix the same thing; I kept this
one because the series is built on it.
2026 Drop the always-true RPR initial flag
The grammar accepts INITIAL or an empty clause and rejects SEEK, so
the field was a constant true, and 2025 left the deparser no longer
reading it. What comes out with it:
- the field itself, from RPCommonSyntax and from WindowClause;
- in gram.y, its %type, the two rule actions that set it, and the
assignment in the RPCommonSyntax action;
- in transformRPR(), the copy into the WindowClause, and the file
header and step comments that named the flag;
- in optimize_window_clauses(), the term it contributed to the
window-dedup equality;
- in README.rpr, its line in the transcription step and in the
structure diagram, which gains rpDefs and reluctant instead.
Most of the diff is the rpr_explain case comparing a window written
with INITIAL against one written without it. It could not fail: a
bare RPR window normalizes to INITIAL, and the test looked only at
the PATTERN line, where the keyword does not appear. A view without
the keyword, printed whole, replaces it.
Best regards,
Henson
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Xuneng Zhou | 2026-08-30 03:04:50 | Re: timeout value overflow in wait for lsn |
| Previous Message | Henson Choi | 2026-08-30 02:03:50 | Re: Row pattern recognition |