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