Re: Row pattern recognition

Hi hackers,

This is an increment on top of v49: it lands the two fixes I left as
still-to-come there -- the DEFINE-evaluation use-after-free, now a dedicated
ExprContext, and the PREV/NEXT/FIRST/LAST namespace collision -- adds a
correctness fix found while reviewing the tuplestore spool (dormant
matches), and applies Jian He's and Tatsuo Ishii's review of the v48 series
as a set of mostly behavior-neutral commits.

Before the patch list, a note on CI: cfbot has been red here, but the
failure is not RPR -- it is the libLLVM 19 + ASAN JIT crash (CF 6870), which
reproduces on plain master. The build-system fix (exclude sanitizer flags
from JIT bitcode generation) is Matheus Alcantara's; the meson half is
v3-0001-Exclude-sanitizer-flags-from-LLVM-JIT-bitcode-gen.patch:

https://postgr.es/m/CAAAe_zBX5uV9K0ikuROLgdNvDCgGqHRskT-73L+oX9=3aXR2AQ@mail.gmail.com

For v50, what would you think about folding that meson patch in as a
temporary prerequisite at the front of the series, so cfbot's ASAN build
gets past the JIT crash and actually exercises RPR?

First, two cleanups splitting changes unrelated to RPR out of the feature
patch:

nocfbot-0001 Remove blank-line changes unrelated to row pattern
recognition
nocfbot-0002 Remove unnecessary includes from the row pattern
recognition patch

The fixes -- the two I left as still-to-come in v48, plus the dormant-match
fix found since (nocfbot-0003..0005, all behavior-changing):

nocfbot-0003 Recognize row pattern navigation operations by name in
DEFINE
The placeholder PREV/NEXT/FIRST/LAST pg_proc functions polluted the
ordinary function namespace and could be silently misbound to
same-named user functions. They are dropped; an unqualified
PREV/NEXT/FIRST/LAST inside a DEFINE clause is recognized as
navigation by name, while a schema-qualified call still reaches an
ordinary function. A navigation operation inside a navigation offset
-- which must be a run-time constant -- is now rejected instead of
crashing the planner. (f).prev follows attribute notation as an
ordinary function, the same inside and outside DEFINE.

nocfbot-0004 Use a dedicated ExprContext for RPR DEFINE clause
evaluation
nocfbot-0039's interim leak fix had turned into a use-after-free and
was voided in v48. DEFINE evaluation now has its own ExprContext,
reset once per row and separate from both the per-output-tuple context
and tmpcontext, resolving the leak and the use-after-free together.

nocfbot-0005 Drive RPR row pattern matching once per row
Matching only advanced when a window function read the frame, so a row
whose only window function skips the frame (e.g. nth_value() with a
NULL offset) left the match behind the current row -- silently wrong
results and a spurious "cannot fetch row ... before WindowObject's
mark
position" error.
The match is now driven once per row, before the window functions run;
the navigation mark advances from the frontier the match reached, so
the tuplestore is trimmed sooner.

Review of v48 (Jian He, and Tatsuo Ishii), as nocfbot-0006..0013 --
behavior-neutral except where tagged [behavior change]:

nocfbot-0006 Tidy up row pattern recognition plumbing
Remove the dead collectPatternVariables()/buildDefineVariableList()
helpers; drop the redundant rpSkipTo/defineClause arguments of
make_windowagg(); mark RPRNavExpr.resulttype query_jumble_ignore;
rename WindowAggState.defineClauseList to defineClauseExprs; assorted
block flattening. No change to planner or executor output.

nocfbot-0007 Further tidy up row pattern recognition plumbing
Drop the now-unused WindowClause argument of transformDefineClause();
use foreach_node()/foreach_current_index() in the DEFINE walkers and
drop their redundant end-of-list break tests; minor include and
comment fixups. No output change.

nocfbot-0008 Refactor transformDefineClause in row pattern recognition
[behavior change]
Hoist the "DEFINE variable not used in PATTERN" cross-check out of the
recursive walker into its caller, and reorder per-variable processing
to transformExpr -> coerce_to_boolean -> pull_var_clause, dropping the
separate second coercion pass. The only observable change is one
error-cursor position: the duplicate-variable error now points at the
later definition. New regression coverage for DEFINE coercion and Var
propagation is added.

nocfbot-0009 Replace a bare block with an else in the RPR DEFINE clause
walker
Cosmetic flattening of define_walker()'s phase dispatch into an
if / else if / else chain.

nocfbot-0010 Rename loop index variables in row pattern deparse helpers
Tatsuo's suggestion; descriptive names for the deparse index/loop
variables, no change to deparsed output.

nocfbot-0011 Rename absorption "judgment point" to "comparison point" in
comments
Comment and executor-README wording only; identifiers unchanged.

nocfbot-0012 Improve comments, documentation, and naming for row pattern
recognition
A batch of comment/doc clarity fixes -- the AST-vs-parse-tree wording,
the Run Condition EXPLAIN test, the contain_rpr_walker comment, the
ALT-marker and quantifier comments, the RPCommonSyntax.location "or
-1"
convention, and the transformDefineClause header -- plus renaming the
saturated-count sentinel RPR_COUNT_MAX to RPR_COUNT_INF for
consistency with RPR_QUANTITY_INF.

nocfbot-0013 Document eval_nav_offset_helper's NULL/negative offset
handling
Comment only. The NEEDS_EVAL offset branch is reachable (a Param
offset can be NULL or negative at run time), so it stays a graceful
return rather than an assertion.

For traceability, where each patch came from:

patch proposer proposal
------------- --------------
-------------------------------------------------
nocfbot-0001 Henson drop blank-line churn unrelated to RPR
nocfbot-0002 Henson drop unnecessary includes
nocfbot-0003 Henson nav namespace collision; (f).prev as
ordinary function
nocfbot-0004 Henson dedicated ExprContext for DEFINE evaluation
nocfbot-0005 Henson drive row pattern matching once per row
nocfbot-0006 Jian He tidy RPR plumbing
nocfbot-0007 Henson further plumbing tidy
nocfbot-0008 Jian He refactor transformDefineClause
nocfbot-0009 Jian He bare block -> else in the DEFINE walker
nocfbot-0010 Tatsuo Ishii rename deparse loop/index variables
nocfbot-0011 Jian He "judgment point" -> "comparison point"
wording
nocfbot-0012 Jian He comment/doc clarity batch + RPR_COUNT_INF
rename
nocfbot-0013 Jian He document eval_nav_offset NULL/negative
offset handling

Still to come -- Jian He's follow-ups on this thread (2026-06-19), to fold
once each approach is agreed (all under review):

topic proposed change
----------------------------- -------------------------------------------
DEFINE qualified column-ref make the differing pg_temp.t.c /
error messages public.t.c / t.c messages consistent, on
the standard "invalid reference to
FROM-clause entry for table"
validateRPRPatternVarCount rename to preprocessRPRPattern
quantifier INF bound abstract behind an RPR_QUANTITY_INF macro
"nullable" variables rename to match_empty
splitRPRTrailingAlt use foreach_node / foreach_current_index
buildRPRPattern signature pass the WindowClause directly
collectDefineVariables / inline and drop the helpers
tryUnwrapSingleChild
variable-limit error msg fold the maximum into the primary message

Quality work to run alongside, on Linux: Valgrind (leak and use-after-free,
to exercise the DEFINE ExprContext fix, whose reproducer is cassert-only)
and standing gcov coverage to catch untested planner paths.

Longer term, and out of scope for this CF entry:

- Smaller documentation and error-message follow-ups (glossary, the
bounded-quantifier message, README wording).
- Short-circuit / tri-state DEFINE evaluation, as a separate series.
- SEEK clause support (SQL:2016).
- Empty pattern PATTERN () -- correctly rejected today; deferred because
its empty-match semantics (SHOW/OMIT EMPTY MATCHES) are tied to the
still-out-of-scope MEASURES clause.
- Relaxing the DEFINE-subquery over-rejection where the standard permits
it (the subquery does no RPR of its own and makes no outer reference).
- Prefix-pattern absorption (an optimization; designed and intentionally
split out as its own series).
- R010 (MATCH_RECOGNIZE in the FROM clause) and the shared RPRContext it
would back.

Please let me know if any of the slicing or grouping looks off.

On a personal note, some fatigue has built up, so I'll be easing off the
pace a little for a while and may be slower to follow up on this thread
than I have been. The work continues -- just at a gentler pace.

Thanks again to Jian and Tatsuo for the careful review.

Best regards,
Henson