Re: Row pattern recognition

On Thu, Jul 9, 2026 at 1:55 PM jian he <jian(dot)universality(at)gmail(dot)com> wrote:
>
>
> That refactor is the patch I posted in [1]. An excerpt from its commit message:
> ```
> Row pattern navigation (PREV/NEXT/FIRST/LAST and their compounds) resolves
> offsets (transforming an expression into an int64) in ExecInitExprRec,
> ExecEvalRPRNavSet, extract_const_offset and eval_define_offsets. Several
> places transform the same offset expression into an int64 -- that is too much.
> With the attached, a single place (ExecInitWindowAgg -> eval_define_offsets)
> handles this.
> ```
>
> I think this belongs in the patchset now, rather than as a follow-up after the
> main patch is committed. Computing the same offset in several places hurts
> readability and, as this bug shows, is error-prone; consolidating it into one
> place makes the code easier to read for future reviewers.
>

Hi.
Still based on https://github.com/assam258-5892/postgres/commits/RPR

Please check the attached v51-0001, v51-0002, v51-0003.
v51-0001 fixes the regression failure
v51-0002: If the target row does not exist, skip evaluating the
navigation function's argument expression
v51-0003: Introduce execution struct RprNavState

For example:

CREATE TABLE t (id int, v int);
INSERT INTO t VALUES (1, 10);
SELECT id, count(*) OVER w AS cnt
FROM t
WINDOW w AS (ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING
PATTERN (A)
DEFINE A AS PREV(v IS NOT NULL) IS NULL);

Here the first row has no PREV row, so the argument expression (v IS NOT NULL)
must not be evaluated at all; PREV(...) itself returns NULL, and PREV(...) IS
NULL is therefore true. The expected cnt value is 1.

To implement this, an extra step EEOP_JUMP_IF_NULL is emitted right after
EEOP_RPR_NAV_SET, jumping over the argument expression's steps directly to
EEOP_RPR_NAV_RESTORE in case of row does not exists.
------------------------------------------------------------------------
Storing the resolved offset values (offset, compound_offset) on RPRNavExpr, next
to the offset expressions they are computed from (offset_arg,
compound_offset_arg),
seems wrong to me: the expressions are plan-tree fields,
but the resolved values are per-execution results, and the executor must treat
the plan tree as read-only.

Compare how FOR PORTION OF handles this in ExecInitModifyTable:

```
exprState = ExecPrepareExpr((Expr *) forPortionOf->targetRange, estate);
targetRange = ExecEvalExpr(exprState, econtext, &isNull);

/* Create state for FOR PORTION OF operation */
fpoState = makeNode(ForPortionOfState);
fpoState->fp_targetRange = targetRange;
```
The expression is evaluated at executor startup, and the resulting value is
stored in an executor-state struct — the plan node is never modified.

We should do the same here: evaluate the offset expressions at executor startup
and store the resulting values in executor state.

So in v51-0003 I added a new executor-state struct, RprNavState, which captures
everything the evaluation code in execExprInterp.c needs: the WindowAggState,
the RPRNavExpr, the resolved offset and compound_offset, and the result type
info (resulttyplen, resulttypbyval). Keeping such per-node evaluation state in
an out-of-line struct, with the ExprEvalStep holding just a pointer to it,
follows an existing convention in the expression evaluation code — see
JsonExprState for a similar example.

--
jian
https://www.enterprisedb.com/