Re: Row pattern recognition

> get_windowclause_startup_tuples
> do we need some comments for WindowClause->defineClause

We already have comments for DEFINE clause. Do you think it's not enough?

	 * Moreover, if row pattern recognition is used, we charge the DEFINE
	 * expressions once per tuple for each variable that appears in PATTERN.

> src/backend/executor/README.rpr
>   (2) Transcription to WindowClause
>       - Copies rpPattern, rpSkipTo, initial fields
> 
> In transformRPR, we did this:
> ``wc->rpPattern = windef->rpCommonSyntax->rpPattern;``
> Since this is an assignment, not a copy, is "Copies rpPattern" incorrect?

I think "Copies" is Ok here. Yes, we do not use copyObject to copy the
rpPattern object but copying just the pointer can be regarded as kind
of "copy".

> ISO reference typo: most occurrences use {ISO/IEC 19075},
> but rpr_integration.sql and some places reference 9075-2:2016 instead of 19075.
> We may need check all occurrences to ensure consistency.

19075 and 9075-2 are the different standard documents.

19075-5: Information technology ― Guidance for the use of database language SQL ― Part 5: Row pattern recognition
9075-2: Information technology ― Database languages ― SQL ― Part 2: Foundation (SQL/Foundation)

IMO they are used differently depending on the context.

> In struct ResultRelInfo, we have fields like {ri_CheckConstraintExprs,
> ri_WithCheckOptionExprs}.
> Similarly, I want to rename WindowAggState->defineClauseList to
> defineClauseExprs

Looks reasonable proposal to me.

> I think the comment on function contain_rpr_walker can be removed,
> since transformWithClause already explains it.

I don't think we want to remove all function comments of
contain_rpr_walker. However I am not against removing "Used by
transformWithClause() to enforce ISO/IEC 9075-2:2016 7.17 SR 3)f) on
WITH RECURSIVE elements." because it's somewhat redundant.

> Is "judgment" really the right word?
> 
> I don't fully understand the purpose of eval_nav_offset_helper now.
> Also, the branch where (isnull == true) and (offset < 0) appears
> unreachable; it's never hit in the regression tests.
> We should add some regression tests for these scenarios.

Yeah. Maybe we can turn them into Assertions?

Regards,
--
Tatsuo Ishii
SRA OSS K.K.
English: http://www.sraoss.co.jp/index_en/
Japanese:http://www.sraoss.co.jp