Re: Row pattern recognition

Hi Jian,

Thanks for the misc-refactoring patch (v48-0001-v48-misc-refactoring) and
the
whole-row Var note.  As before, now that v48 is posted I'll rebase on it
and send
these as incremental patches on top (v49); per-item disposition below, with
the
per-commit breakdown to come with the patch.


> RPRNavExpr->resulttype should also marked as
pg_node_attr(query_jumble_ignore)

Agreed -- every other derived result-type field in primnodes.h already
carries it,
and resulttype is derived from the (jumbled) arg type, so ignoring it loses
nothing.
I'll add it.


> collectPatternVariables is not needed.
> The parser already ensures every DEFINE variable appears in PATTERN ...
> See nfa_evaluate_row the for loop break.

Confirmed -- the parser already rejects a DEFINE variable not used in
PATTERN, so
every DEFINE var is in PATTERN; filtering is redundant and cost_windowagg
can
iterate defineClause directly, so I'll remove it.


> buildDefineVariableList is trivial. No need to export it as an external
function.

Agreed -- I'll drop it and inline the list-building into
create_windowagg_plan().


> Rename WindowAggState.defineClauseList to defineClauseExprs

Agreed (the elements are ExprState, so it matches the ...Exprs convention)
-- I'll
do it, including the additional site the dormant-match fix touches.


> Flatten a needlessly nested block in show_window_def().
> Replace a post-loop ListCell NULL check in
remove_unused_subquery_outputs()
> with a boolean flag.
> Reduce the number of arguments in make_windowagg.
> Minor refactoring of regress test comments.

I'll do all of these.

One exception: the parsenodes.h RPRPatternNode comment names the data
structure, so
rather than "clause" I'll change it to "parse tree node".


> dvar, var both can be whole-row Vars, but this seems to work for
whole-row vars.
> We need some simple regress tests for cases where both are whole-row vars
or one
> of them is a whole-row var.

Good catch -- I'll add regress coverage for the whole-row Var cases (both
sides
whole-row, and one side whole-row) in the needed_by_define check, in v49.

I'll send the items above as a first incremental patch on top of v48, and
take up
the later reviews after that.


Thanks,
Henson