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