Re: Row pattern recognition

Hi Jian,

Thank you for 0001 and 0002. Both are clean changes, and I would
like to take them as they are.

> 0001: Remove RPRNavExpr.navno
> navno spans all three stages: ParseRPRNavCall() initializes it,
> compute_define_metadata() compute it while planning, and
> ExecInitExprRec() use it. Tracking and keeping those in sync is
> not trivial.
> It's doable to just remove it.

I like this a lot. The number only existed to find an entry in
rprNavOffsets, which needed the planner and the executor to walk
the navigations in the same order. Registering the entry right in
ExecInitExprRec() while the DEFINE clause is compiled, the way
WindowFunc is registered in winstate->funcs, is the right place
for it: there is nothing left to keep in step, and
nav_offsets_walker() and build_nav_offsets() go away with it.

> 0002: Remove compute_define_metadata
> IMHO compute_define_metadata() is complicated.
> Instead, loop over the DEFINE clause with foreach, run an
> expression walker on each variable's expression to check whether
> it depends on the match start, and if it does, record that
> variable's index.

Agreed. Without navno the context struct and the per-variable
bookkeeping have no job left, and a bool walker with a foreach_node()
loop says the same thing in far fewer lines.

I applied both and checked them: no build warnings, the regression
tests pass (also with debug_copy_parse_plan_trees and
debug_write_read_parse_plan_trees on), and 575 DEFINE shapes give
the same rows, errors, EXPLAIN output and absorption markers as a
build without the patches.

> Let's agree to disagree, at least now, removing
> WindowAggState.nfaStateSize is not performance related.

Fair enough, thank you.

> Also as mentioned in
> https://www.postgresql.org/message-id/688312.1782222788%40sss.pgh.pa.us
> we should use size_t, not using Size

Understood. I will switch the remaining Size uses in the RPR code
to size_t.

Once v54 is posted, I will merge 0001 and 0002 into the patch
series at a relaxed pace, after more quality checks and a more
detailed review.

A small request about naming. Your v54-0001 and v54-0002 share
their names with the series, and each new batch starts again at
0001, so they are hard to tell apart when I merge them. Could you
name your incremental patches by feature and number them within
each feature, for example nocfbot-feat1-0001, nocfbot-feat1-0002
and nocfbot-feat2-0001, so that the series keeps its own numbering?

Best regards,
Henson