Re: Row pattern recognition

Hi Henson,

Thank you for the patches. I looked into 0013.

> diff --git a/src/backend/parser/parse_cte.c b/src/backend/parser/parse_cte.c
> index ccde199319a..3830597bb2b 100644
> --- a/src/backend/parser/parse_cte.c
> +++ b/src/backend/parser/parse_cte.c
> @@ -96,6 +96,14 @@ static void checkWellFormedRecursion(CteState *cstate);
> static bool checkWellFormedRecursionWalker(Node *node, CteState *cstate);
> static void checkWellFormedSelectStmt(SelectStmt *stmt, CteState *cstate);
>
> +/* Recursive-WITH RPR rejection */
> +typedef struct
> +{
> + ParseLoc location; /* location of first RPR window, or -1 */
> +} ContainRPRContext;
> +
> +static bool contain_rpr_walker(Node *node, void *context);
> +
>
> /*
> * transformWithClause -
> @@ -157,13 +165,31 @@ transformWithClause(ParseState *pstate, WithClause *withClause)
> if (withClause->recursive)
> {
> /*
> - * For WITH RECURSIVE, we rearrange the list elements if needed to
> - * eliminate forward references. First, build a work array and set up
> - * the data structure needed by the tree walkers.

I think removing the exiting comment ("For WITH RECURSIVE, we rearrange
the list elements...") is not appropriate as this explains subsequent
process, which is not changed after the patch.

> + * Per ISO/IEC 9075-2:2016 7.17 Syntax Rule 3)e)f), every <with list
> + * element> in a WITH RECURSIVE clause is "potentially recursive" and
> + * shall not contain a <row pattern common syntax>. (PostgreSQL does
> + * not implement <row pattern measures>, so only the common syntax
> + * needs to be checked.) ISO/IEC 19075-5 6.17.5 (R020) and 4.18.5
> + * (R010) restate the prohibition for CREATE RECURSIVE VIEW, which is
> + * rewritten to WITH RECURSIVE by makeRecursiveViewSelect() and so
> + * flows through here as well.
> */
> CteState cstate;
> int i;
>
> + foreach(lc, withClause->ctes)
> + {
> + CommonTableExpr *cte = (CommonTableExpr *) lfirst(lc);
> + ContainRPRContext ctx;
> +
> + ctx.location = -1;
> + if (contain_rpr_walker(cte->ctequery, &ctx))
> + ereport(ERROR,
> + errcode(ERRCODE_FEATURE_NOT_SUPPORTED),

ERRCODE_FEATURE_NOT_SUPPORTED should be ERRCODE_SYNTAX_ERROR instead?
IMO ERRCODE_FEATURE_NOT_SUPPORTED is used when the feature is defined
by the standard but PostgreSQL just has not implemented yet. In this
case the standard disllow RPR in recursive CTE.

> + errmsg("cannot use row pattern recognition in a recursive query"),
> + parser_errposition(pstate, ctx.location));
> + }
> +
> cstate.pstate = pstate;
> cstate.numitems = list_length(withClause->ctes);
> cstate.items = (CteItem *) palloc0(cstate.numitems * sizeof(CteItem));
> @@ -1268,3 +1294,29 @@ checkWellFormedSelectStmt(SelectStmt *stmt, CteState *cstate)
> }
> }
> }

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