Re: Row pattern recognition

Hi Jian,

Thanks for this -- consolidating the offset resolution into one place
is a direction I like, and a couple of things about it are genuinely
nice:

> EXPLAIN reads the resolved offsets from the planstate
> (ExecInitWindowAgg is reached for EXPLAIN without ANALYZE), so it now
> prints concrete offsets instead of "runtime".

That is a real improvement -- the offset becomes visible in the plan
even when it comes from a bind parameter, where today we can only show
"runtime".

It also lines up with a point Tatsuo made back in April: since the
planner already folds a constant offset expression down to a single
Const, doing the resolution in the executor means we no longer have to
care whether the offset was constant or not.  So the direction has his
backing as well.

That said, I don't think we should take it into the patch at this
stage.  The current version has one proven defect: a plain
FIRST(expr, n) with n > 0 loses its forward reach.  I added a test for
it -- "DEFINE A AS v > FIRST(v, 5)" in rpr_explain -- where the
baseline prints "Nav Mark Lookahead: 5" and the patch prints 0.  The
query result is still correct (the mark is under-advanced, so it only
keeps extra rows), but the EXPLAIN is wrong and the tuplestore trim is
lost, so it is a regression against the baseline.

The test will come with the next patch email I send, so you will have
it there to diff against.

More broadly, though, reshaping the offset handling this much right as
we are stabilizing for commit feels like the wrong time -- at this
point I would rather limit changes to minimal fixes for proven
defects.  This is a worthwhile cleanup that deserves proper review
time, and it would be more valuable if it can be refined without that
pressure.

So how about, once the current patch is verified and committed, we
pursue this as a separate follow-up?

Best regards,
Henson