Re: Row pattern recognition

Hi Tatsuo,

Thank you for the feedback.

Note that applying 0001-0006 produced compiler warning.
>
> execExprInterp.c: In function ‘ExecEvalRPRNavSet’:
> execExprInterp.c:6005:23: warning: ‘target_pos’ may be used uninitialized
> [-Wmaybe-uninitialized]
> 6005 | target_slot = ExecRPRNavGetSlot(winstate, target_pos);
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> execExprInterp.c:5959:25: note: ‘target_pos’ was declared here
> 5959 | int64 target_pos;
> | ^~~~~~~~~~
>

`ExecRPRNavGetSlot()`, along with the enclosing `ExecEvalRPRNavSet()`
and its `int64 target_pos` declaration, is introduced by nocfbot-0017
(Implement 1-slot PREV/NEXT navigation for RPR), not at the 0001-0006
level. Could you double-check the actual applied range on your side?
The function simply does not exist at 0001-0006.

Since the function is not present at 0001-0006, I suspect the
applied patch set on your side is in a different state than
expected. Could you verify the range and contents of the patches
actually applied?

But regression test was ok.
> However, after applying 0007, regression test failed.
>
> not ok 196 + rpr_integration 514 ms
> # (test process exited with exit code 2)
>

nocfbot-0006 was written specifically to prevent the crashes that
0007's tests expose, so with 0001-0007 applied in order the issue
should already be resolved. On my authoritative base (cfbot/4460) all
rpr_integration tests pass through 0008. I had been routinely
building only on macOS ARM64 recently, so I just re-verified on
Rocky 9 and Ubuntu 24 AMD64 as well; no build warning and no
rpr_integration failure on either.

For reference, here is my working tree layout. Titles only for my
post-base commits (hashes on my side won't match yours); base
hashes are public and directly comparable on your side:

[post-base, newest first]
31. Remove unused include and fix header ordering in RPR files
(origin/RPR)
30. Add inline comments for complex RPR algorithms and design notes
29. Fix nav_slot pass-by-ref dangling pointer in RPR navigation
28. Fix RPR documentation: synopsis, grammar, and terminology
27. Fix comment typos, grammar, and inaccuracies in RPR code
26. Fix RPR error message style: hint format, terminology, capitalization
25. Guard against int64 overflow in RPR bounded frame end computation
24. Implement FIRST/LAST and compound navigation for RPR
23. Add Nav Mark Lookback to EXPLAIN and fix compute_nav_max_offset()
22. Add 2-arg PREV/NEXT test for row pattern navigation with host
variable
21. Enable JIT compilation for PREV/NEXT navigation tests in RPR
20. Update RPR code comments to reflect 1-slot navigation model
19. Add tuplestore trim optimization for RPR PREV navigation
18. Add JIT compilation support for RPR PREV/NEXT navigation
17. Implement 1-slot PREV/NEXT navigation for RPR
16. Normalize RPR element flag macros to return bool
15. Narrow variable scope in ExecInitWindowAgg DEFINE clause loop
14. Add CHECK_FOR_INTERRUPTS to RPR context cleanup and finalize loops
13. Remove unused force_colno parameter from RPR deparse functions
12. Fix execRPR.o ordering in executor Makefile to match meson.build
11. Fix quote_identifier() for RPR pattern variable name deparse
10. Rename rpr_explain test views from sequential numbers to descriptive
names
9. Add fixed-length group absorption for RPR
8. Replace reduced frame map with single match result
7. Add RPR planner integration tests
6. Fix DEFINE expression handling in RPR window planning
5. Fix mark handling for last_value() under RPR
4. Fix in-place modification of defineClause TargetEntry in setrefs.c
3. Add CHECK_FOR_INTERRUPTS() to nfa_try_absorb_context() loop
2. Add CHECK_FOR_INTERRUPTS() to nfa_add_state_unique() for state
explosion patterns
1. Remove unused regex/regex.h include from nodeWindowAgg.c

[base, public hashes]
732acf9b7c6 (cfbot/cf/4460) [CF 4460] v46 - Implement row pattern
recognition feature
6eb8841afaf Row pattern recognition patch (typedefs.list).
5213dbb3c4f Row pattern recognition patch (tests: expected).
bdc3a17982b Row pattern recognition patch (tests: SQL, data).
596bcc434b5 Row pattern recognition patch (docs).
e02872fd59c Row pattern recognition patch (executor and commands).
742c6eb673e Row pattern recognition patch (planner).
8514ca64f2d Row pattern recognition patch (rewriter).
d26741d4f1d Row pattern recognition patch (parse/analysis).
f85d2ee8f4a Row pattern recognition patch for raw parser.
e484b0eea61 (origin/RPR-base, RPR-base) Fix two issues in fast-path FK
check introduced by commit 2da86c1ef9

Could you send me the equivalent `git log` for your working tree
covering the full v46 application and the commit immediately
preceding it? In particular, could you also check whether the
recent master commit titled "Fix integer overflow in
nodeWindowAgg.c" (backpatched to 14) is included in your base? It
touches the same ROWS/GROUPS frame boundary code path that RPR
interacts with, so its presence or absence will help narrow things
down. Once I can match the same base locally, I will reproduce and
include the fix in the next revision.

A quick status update:

- nocfbot-0006: self-review complete, not yet sent (comment tweak per
review)
- nocfbot-0007: under self-review (comment details per review)
- nocfbot-0008: in progress

I would like to resolve the crash first before sending the next
revision, so that the full set goes out together once our bases are
aligned and A3 passes on your side.

One process question: so far I have been folding review feedback
into the relevant existing patches (i.e. amending nocfbot-0008 to
reflect your comments on it). Would you prefer that I instead
append the review fixes as new patches at the end of the series
(e.g. nocfbot-0032, 0033, ...), so the history of reviewed changes
stays visible? Happy to follow whichever workflow you find easier
to review.

Regards,
Henson