Re: Row pattern recognition

Hi Tatsuo,

Thank you for the review and the attached patch for 0005.
I appreciate you taking the time to look at each patch
carefully.

Attached are 8 incremental patches on top of v46 (replacing
the previous 6-patch set). 0001-0003 are unchanged. 0004 and
0005 incorporate your feedback below. 0006 is a new planner
fix. Previous 0006 becomes 0007. 0008 is new.

> 0004: Fix in-place modification of defineClause TargetEntry
>
> Probably we want to modify the comment above since it implies an
> in-place modification? What about something like this? (Modifies ->
> Replace)
>
> /*
> * Replace an expression tree in each DEFINE clause so that all Var
> * nodes's varno refers to OUTER_VAR.
> */
>

Good point, thank you. Applied in the updated 0004.

> > 0005: Fix mark handling for last_value() under RPR
>
> I think instead we can set a mark at frameheadpos when seek type is
> SEEK_TAIL and RPR is enabled. See attached patch.
>

Thank you for the patch. Your approach is cleaner -- setting
the mark at frameheadpos is more direct than suppressing
advancement. I've adopted it in the updated 0005.

> > 0006: Implement 1-slot PREV/NEXT navigation for RPR
>
> Excellent! I will take a look at it. (it will take for a while).
>

No rush at all. In the meantime, while testing the PREV/NEXT
patch in various query patterns, I found a planner issue.
I've also added JIT support. Here is a summary of the new
patches:

0006: Prevent removal of RPR window functions in unused
subquery outputs

Wrapping an RPR window query in a subquery with an
outer aggregate causes a crash:

SELECT count(*) FROM (
SELECT count(*) OVER w FROM ...
WINDOW w AS (... DEFINE A AS i > PREV(i))
) t;

remove_unused_subquery_outputs() replaces unused subquery
target entries with NULL constants. When an RPR window
function's result is not referenced by the outer query,
this replacement eliminates all active window functions
for the WindowClause, causing the planner to omit the
WindowAgg node. DEFINE clause expressions containing
RPRNavExpr (PREV/NEXT) then lose their execution context,
leading to a crash.

The fix skips the NULL replacement for window functions
whose WindowClause has a defineClause. Even when the
window function result is unused, RPR pattern matching
(frame reduction) must still execute -- the WindowAgg
node must be preserved.

An alternative would be to let the planner remove the
window function but teach it to still generate the
WindowAgg node when defineClause is present, even with
no active window functions. That would be a more
targeted optimization but requires deeper planner
changes. Do you think the current approach is
sufficient, or would you prefer a different strategy?

0007: Implement 1-slot PREV/NEXT navigation for RPR
(unchanged from previous 0006)

> > 2. LLVM JIT fallback (in 0006): The mid-expression slot swap
>
> I am not an expert of JIT, but for me, it sounds reasonable. We can
> enhance it later on.
>

0008: Add JIT compilation support for RPR PREV/NEXT navigation

EEOP_OUTER_VAR normally uses slot pointers cached in the
entry block, but the mid-expression slot swap in
NAV_SET/RESTORE invalidates them. To avoid penalizing
existing expressions, the reload path is only generated
when RPR navigation opcodes are present in the
expression (compile-time decision). Expressions without
PREV/NEXT produce identical machine code as before.
NAV_SET/RESTORE use the standard build_EvalXFunc pattern.

A 100K-row PREV/NEXT test case that runs under
jit_above_cost=0 is included in rpr.sql.

I would appreciate your thoughts on these when you have time.

Best regards,
Henson