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


Reply via email to