> 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
