| From: | Tatsuo Ishii <ishii(at)postgresql(dot)org> |
|---|---|
| To: | assam258(at)gmail(dot)com |
| 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, pgsql-hackers(at)postgresql(dot)org |
| Subject: | Re: Row pattern recognition |
| Date: | 2026-03-22 05:23:26 |
| Message-ID: | 20260322.142326.546998002680149319.ishii@postgresql.org |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
>> Agreed. So I just want to make sure I create v46 using 0001-0003 and
>> 0005-0015. Am I correct?
>
>
> Yes, that’s right!
Ok, attached is the v46 patches. Here some comments:
- As git apply did not like 0003, I applied it using patch command.
- 0010 did not apply because 0004 skipped. I modified 0010 a little
bit and applied it.
- Henson's "experimental" implementation of PREV/NEXT are not included
in the patch set.
Here are differences from v45.
- Number of patch files are now 9, not 8. As we discussed, I split
the regression test patches into two separate patch files: SQL +
data patches and expected files. It seems CFBot does not work
smoothly if a patch file is too large. We hope the splitting makes
CFBot work more smoothly.
Below are comments from Henson. See
https://www.postgresql.org/message-id/CAAAe_zBbrnx2fjK2s%2BJgx6TSOdnKAPawXbHeX49WqmX9ji%2BHdg%40mail.gmail.com
for more details.
0001: Fix mergeGroupPrefixSuffix() max increment
When absorbing prefix/suffix, child->max was not incremented,
causing incorrect quantifier bounds. (reported by Zsolt)
0002: Fix RPR error codes and GROUPS typo
Use ERRCODE_WINDOWING_ERROR instead of ERRCODE_SYNTAX_ERROR
for semantic window-frame violations. Fix "GROUP" typo to
"GROUPS" in frame validation error message. (reported by Zsolt)
0003: Add check_stack_depth() and CHECK_FOR_INTERRUPTS()
Add stack depth protection to recursive pattern optimization
and interrupt checks to NFA engine loops.
0005: Fix row_is_in_reduced_frame in WINDOW_SEEK_TAIL
Pass frameheadpos directly to row_is_in_reduced_frame
instead of frameheadpos+relpos. Currently only last_value()
calls this path with relpos=0 so no actual bug, but the
old expression would be incorrect for negative relpos.
Also add a bounds check for future callers.
0006: Clarify ST_NONE intent
Add comment explaining ST_NONE = 0 is the default for
non-RPR windows.
0007: Clarify inverse transition optimization comment
Document why RPR disables inverse transition: the reduced
frame changes row by row.
0008: Reject unused DEFINE variables
I know you preferred silently ignoring unused DEFINE
variables [1], and I agree the standard doesn't mandate
an error. However, if we later add qualified column
references (e.g. B AS A.price > 100), B's expression
depends on A being present. If A is not used in PATTERN
and the planner silently removes it, B's reference to A
becomes dangling. I worry that silently allowing this now
could create forward-compatibility problems once qualified
references are introduced. For that reason, I'm inclined
to think rejecting them now may be safer than changing
behavior later, which would be a user-visible compatibility
break. This is also consistent with Oracle's behavior
(ORA-62503), as SungJun reported, and it helps catch user
typos in DEFINE variable names at parse time.
0009: Clarify RPR documentation in advanced.sgml
Improve absorption explanation and clarify non-match row
aggregation behavior with concrete examples.
0010: Fix typos in RPR comments and parser README
0011: Clarify excludeLocation and empty quantifier in gram.y
Add comments explaining the conditional location assignment
pattern and the empty quantifier rule.
0012: Clarify RPR_VARID_MAX definition
Document varId range 0-250 and reserved control element
values 252+.
0013: Move local variables to function scope
In row_is_in_reduced_frame, move declarations out of
switch case blocks.
0014: Reset reduced_frame_map pointer in release_partition
Set reduced_frame_map = NULL and alloc_sz = 0 to prevent
dangling pointer after partition context reset.
0015: Remove redundant list manipulation in nfa_add_matched_state
Simplify doubly-linked list operation that was duplicated
by the subsequent ExecRPRFreeContext() call.
Best regards,
--
Tatsuo Ishii
SRA OSS K.K.
English: http://www.sraoss.co.jp/index_en/
Japanese:http://www.sraoss.co.jp
| Attachment | Content-Type | Size |
|---|---|---|
| v46-0001-Row-pattern-recognition-patch-for-raw-parser.patch | application/octet-stream | 33.7 KB |
| v46-0002-Row-pattern-recognition-patch-parse-analysis.patch | application/octet-stream | 31.8 KB |
| v46-0003-Row-pattern-recognition-patch-rewriter.patch | application/octet-stream | 5.8 KB |
| v46-0004-Row-pattern-recognition-patch-planner.patch | application/octet-stream | 73.0 KB |
| v46-0005-Row-pattern-recognition-patch-executor-and-comma.patch | application/octet-stream | 165.4 KB |
| v46-0006-Row-pattern-recognition-patch-docs.patch | application/octet-stream | 16.4 KB |
| v46-0007-Row-pattern-recognition-patch-tests-SQL-data.patch | application/octet-stream | 398.1 KB |
| v46-0008-Row-pattern-recognition-patch-tests-expected.patch | application/octet-stream | 568.1 KB |
| v46-0009-Row-pattern-recognition-patch-typedefs.list.patch | application/octet-stream | 1.1 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | John Naylor | 2026-03-22 05:26:24 | Re: vectorized CRC on ARM64 |
| Previous Message | Tom Lane | 2026-03-22 04:24:47 | Re: pg_waldump: support decoding of WAL inside tarfile |