Fix inability to reference CYCLE column from inside its CTE.

Such references failed with "cache lookup failed for type 0"
because we didn't resolve the type of the CYCLE column until after
analyzing the CTE's query.  We can just move that processing
to before the recursive parse_sub_analyze call, though.

While here, invent a couple of local variables to make this
code less egregiously wider-than-80-columns.

Per bug #17723 from Vik Fearing.  Back-patch to v14 where
the CYCLE feature was added.

Discussion: https://postgr.es/m/17723-2c4985ff111e7bba@postgresql.org

diff --git a/src/backend/parser/parse_cte.c b/src/backend/parser/parse_cte.c
index f6ae96333af79f88caeaf496db628c8acf78447a..ac263af07e74516da7a26b9116d52c7af2680fcc 100644 (file)
--- a/src/backend/parser/parse_cte.c
+++ b/src/backend/parser/parse_cte.c
@@ -241,10 +241,76 @@ static void
 analyzeCTE(ParseState *pstate, CommonTableExpr *cte)
 {
    Query      *query;
+   CTESearchClause *search_clause = cte->search_clause;
+   CTECycleClause *cycle_clause = cte->cycle_clause;
 
    /* Analysis not done already */
    Assert(!IsA(cte->ctequery, Query));
 
+   /*
+    * Before analyzing the CTE's query, we'd better identify the data type of
+    * the cycle mark column if any, since the query could refer to that.
+    * Other validity checks on the cycle clause will be done afterwards.
+    */
+   if (cycle_clause)
+   {
+       TypeCacheEntry *typentry;
+       Oid         op;
+
+       cycle_clause->cycle_mark_value =
+           transformExpr(pstate, cycle_clause->cycle_mark_value,
+                         EXPR_KIND_CYCLE_MARK);
+       cycle_clause->cycle_mark_default =
+           transformExpr(pstate, cycle_clause->cycle_mark_default,
+                         EXPR_KIND_CYCLE_MARK);
+
+       cycle_clause->cycle_mark_type =
+           select_common_type(pstate,
+                              list_make2(cycle_clause->cycle_mark_value,
+                                         cycle_clause->cycle_mark_default),
+                              "CYCLE", NULL);
+       cycle_clause->cycle_mark_value =
+           coerce_to_common_type(pstate,
+                                 cycle_clause->cycle_mark_value,
+                                 cycle_clause->cycle_mark_type,
+                                 "CYCLE/SET/TO");
+       cycle_clause->cycle_mark_default =
+           coerce_to_common_type(pstate,
+                                 cycle_clause->cycle_mark_default,
+                                 cycle_clause->cycle_mark_type,
+                                 "CYCLE/SET/DEFAULT");
+
+       cycle_clause->cycle_mark_typmod =
+           select_common_typmod(pstate,
+                                list_make2(cycle_clause->cycle_mark_value,
+                                           cycle_clause->cycle_mark_default),
+                                cycle_clause->cycle_mark_type);
+
+       cycle_clause->cycle_mark_collation =
+           select_common_collation(pstate,
+                                   list_make2(cycle_clause->cycle_mark_value,
+                                              cycle_clause->cycle_mark_default),
+                                   true);
+
+       /* Might as well look up the relevant <> operator while we are at it */
+       typentry = lookup_type_cache(cycle_clause->cycle_mark_type,
+                                    TYPECACHE_EQ_OPR);
+       if (!OidIsValid(typentry->eq_opr))
+           ereport(ERROR,
+                   errcode(ERRCODE_UNDEFINED_FUNCTION),
+                   errmsg("could not identify an equality operator for type %s",
+                          format_type_be(cycle_clause->cycle_mark_type)));
+       op = get_negator(typentry->eq_opr);
+       if (!OidIsValid(op))
+           ereport(ERROR,
+                   errcode(ERRCODE_UNDEFINED_FUNCTION),
+                   errmsg("could not identify an inequality operator for type %s",
+                          format_type_be(cycle_clause->cycle_mark_type)));
+
+       cycle_clause->cycle_mark_neop = op;
+   }
+
+   /* Now we can get on with analyzing the CTE's query */
    query = parse_sub_analyze(cte->ctequery, pstate, cte, false, true);
    cte->ctequery = (Node *) query;
 
@@ -339,7 +405,10 @@ analyzeCTE(ParseState *pstate, CommonTableExpr *cte)
            elog(ERROR, "wrong number of output columns in WITH");
    }
 
-   if (cte->search_clause || cte->cycle_clause)
+   /*
+    * Now make validity checks on the SEARCH and CYCLE clauses, if present.
+    */
+   if (search_clause || cycle_clause)
    {
        Query      *ctequery;
        SetOperationStmt *sos;
@@ -386,12 +455,12 @@ analyzeCTE(ParseState *pstate, CommonTableExpr *cte)
                     errmsg("with a SEARCH or CYCLE clause, the right side of the UNION must be a SELECT")));
    }
 
-   if (cte->search_clause)
+   if (search_clause)
    {
        ListCell   *lc;
        List       *seen = NIL;
 
-       foreach(lc, cte->search_clause->search_col_list)
+       foreach(lc, search_clause->search_col_list)
        {
            Value      *colname = lfirst(lc);
 
@@ -400,33 +469,31 @@ analyzeCTE(ParseState *pstate, CommonTableExpr *cte)
                        (errcode(ERRCODE_SYNTAX_ERROR),
                         errmsg("search column \"%s\" not in WITH query column list",
                                strVal(colname)),
-                        parser_errposition(pstate, cte->search_clause->location)));
+                        parser_errposition(pstate, search_clause->location)));
 
            if (list_member(seen, colname))
                ereport(ERROR,
                        (errcode(ERRCODE_DUPLICATE_COLUMN),
                         errmsg("search column \"%s\" specified more than once",
                                strVal(colname)),
-                        parser_errposition(pstate, cte->search_clause->location)));
+                        parser_errposition(pstate, search_clause->location)));
            seen = lappend(seen, colname);
        }
 
-       if (list_member(cte->ctecolnames, makeString(cte->search_clause->search_seq_column)))
+       if (list_member(cte->ctecolnames, makeString(search_clause->search_seq_column)))
            ereport(ERROR,
                    errcode(ERRCODE_SYNTAX_ERROR),
                    errmsg("search sequence column name \"%s\" already used in WITH query column list",
-                          cte->search_clause->search_seq_column),
-                   parser_errposition(pstate, cte->search_clause->location));
+                          search_clause->search_seq_column),
+                   parser_errposition(pstate, search_clause->location));
    }
 
-   if (cte->cycle_clause)
+   if (cycle_clause)
    {
        ListCell   *lc;
        List       *seen = NIL;
-       TypeCacheEntry *typentry;
-       Oid         op;
 
-       foreach(lc, cte->cycle_clause->cycle_col_list)
+       foreach(lc, cycle_clause->cycle_col_list)
        {
            Value      *colname = lfirst(lc);
 
@@ -435,97 +502,54 @@ analyzeCTE(ParseState *pstate, CommonTableExpr *cte)
                        (errcode(ERRCODE_SYNTAX_ERROR),
                         errmsg("cycle column \"%s\" not in WITH query column list",
                                strVal(colname)),
-                        parser_errposition(pstate, cte->cycle_clause->location)));
+                        parser_errposition(pstate, cycle_clause->location)));
 
            if (list_member(seen, colname))
                ereport(ERROR,
                        (errcode(ERRCODE_DUPLICATE_COLUMN),
                         errmsg("cycle column \"%s\" specified more than once",
                                strVal(colname)),
-                        parser_errposition(pstate, cte->cycle_clause->location)));
+                        parser_errposition(pstate, cycle_clause->location)));
            seen = lappend(seen, colname);
        }
 
-       if (list_member(cte->ctecolnames, makeString(cte->cycle_clause->cycle_mark_column)))
+       if (list_member(cte->ctecolnames, makeString(cycle_clause->cycle_mark_column)))
            ereport(ERROR,
                    errcode(ERRCODE_SYNTAX_ERROR),
                    errmsg("cycle mark column name \"%s\" already used in WITH query column list",
-                          cte->cycle_clause->cycle_mark_column),
-                   parser_errposition(pstate, cte->cycle_clause->location));
-
-       cte->cycle_clause->cycle_mark_value = transformExpr(pstate, cte->cycle_clause->cycle_mark_value,
-                                                           EXPR_KIND_CYCLE_MARK);
-       cte->cycle_clause->cycle_mark_default = transformExpr(pstate, cte->cycle_clause->cycle_mark_default,
-                                                             EXPR_KIND_CYCLE_MARK);
+                          cycle_clause->cycle_mark_column),
+                   parser_errposition(pstate, cycle_clause->location));
 
-       if (list_member(cte->ctecolnames, makeString(cte->cycle_clause->cycle_path_column)))
+       if (list_member(cte->ctecolnames, makeString(cycle_clause->cycle_path_column)))
            ereport(ERROR,
                    errcode(ERRCODE_SYNTAX_ERROR),
                    errmsg("cycle path column name \"%s\" already used in WITH query column list",
-                          cte->cycle_clause->cycle_path_column),
-                   parser_errposition(pstate, cte->cycle_clause->location));
+                          cycle_clause->cycle_path_column),
+                   parser_errposition(pstate, cycle_clause->location));
 
-       if (strcmp(cte->cycle_clause->cycle_mark_column,
-                  cte->cycle_clause->cycle_path_column) == 0)
+       if (strcmp(cycle_clause->cycle_mark_column,
+                  cycle_clause->cycle_path_column) == 0)
            ereport(ERROR,
                    errcode(ERRCODE_SYNTAX_ERROR),
                    errmsg("cycle mark column name and cycle path column name are the same"),
-                   parser_errposition(pstate, cte->cycle_clause->location));
-
-       cte->cycle_clause->cycle_mark_type = select_common_type(pstate,
-                                                               list_make2(cte->cycle_clause->cycle_mark_value,
-                                                                          cte->cycle_clause->cycle_mark_default),
-                                                               "CYCLE", NULL);
-       cte->cycle_clause->cycle_mark_value = coerce_to_common_type(pstate,
-                                                                   cte->cycle_clause->cycle_mark_value,
-                                                                   cte->cycle_clause->cycle_mark_type,
-                                                                   "CYCLE/SET/TO");
-       cte->cycle_clause->cycle_mark_default = coerce_to_common_type(pstate,
-                                                                     cte->cycle_clause->cycle_mark_default,
-                                                                     cte->cycle_clause->cycle_mark_type,
-                                                                     "CYCLE/SET/DEFAULT");
-
-       cte->cycle_clause->cycle_mark_typmod = select_common_typmod(pstate,
-                                                                   list_make2(cte->cycle_clause->cycle_mark_value,
-                                                                              cte->cycle_clause->cycle_mark_default),
-                                                                   cte->cycle_clause->cycle_mark_type);
-
-       cte->cycle_clause->cycle_mark_collation = select_common_collation(pstate,
-                                                                         list_make2(cte->cycle_clause->cycle_mark_value,
-                                                                                    cte->cycle_clause->cycle_mark_default),
-                                                                         true);
-
-       typentry = lookup_type_cache(cte->cycle_clause->cycle_mark_type, TYPECACHE_EQ_OPR);
-       if (!typentry->eq_opr)
-           ereport(ERROR,
-                   errcode(ERRCODE_UNDEFINED_FUNCTION),
-                   errmsg("could not identify an equality operator for type %s",
-                          format_type_be(cte->cycle_clause->cycle_mark_type)));
-       op = get_negator(typentry->eq_opr);
-       if (!op)
-           ereport(ERROR,
-                   errcode(ERRCODE_UNDEFINED_FUNCTION),
-                   errmsg("could not identify an inequality operator for type %s",
-                          format_type_be(cte->cycle_clause->cycle_mark_type)));
-
-       cte->cycle_clause->cycle_mark_neop = op;
+                   parser_errposition(pstate, cycle_clause->location));
    }
 
-   if (cte->search_clause && cte->cycle_clause)
+   if (search_clause && cycle_clause)
    {
-       if (strcmp(cte->search_clause->search_seq_column,
-                  cte->cycle_clause->cycle_mark_column) == 0)
+       if (strcmp(search_clause->search_seq_column,
+                  cycle_clause->cycle_mark_column) == 0)
            ereport(ERROR,
                    errcode(ERRCODE_SYNTAX_ERROR),
                    errmsg("search sequence column name and cycle mark column name are the same"),
-                   parser_errposition(pstate, cte->search_clause->location));
+                   parser_errposition(pstate, search_clause->location));
 
-       if (strcmp(cte->search_clause->search_seq_column,
-                  cte->cycle_clause->cycle_path_column) == 0)
+       if (strcmp(search_clause->search_seq_column,
+                  cycle_clause->cycle_path_column) == 0)
            ereport(ERROR,
                    errcode(ERRCODE_SYNTAX_ERROR),
                    errmsg("search sequence column name and cycle path column name are the same"),
-                   parser_errposition(pstate, cte->search_clause->location));
+                   parser_errposition(pstate, search_clause->location));
    }
 }
 

diff --git a/src/test/regress/expected/with.out b/src/test/regress/expected/with.out
index 8e5e31a7be68aa1680d3dc411261459cb637939e..5ccadb504214fa06af8b8ae87e60c66c2d0d7b53 100644 (file)
--- a/src/test/regress/expected/with.out
+++ b/src/test/regress/expected/with.out
@@ -1221,6 +1221,29 @@ select * from test;
  0 | t        | {(0),(1),(2),(3),(4),(5),(6),(7),(8),(9),(0)}
 (11 rows)
 
+with recursive test as (
+  select 0 as x
+  union all
+  select (x + 1) % 10
+  from test
+    where not is_cycle  -- redundant, but legal
+) cycle x set is_cycle using path
+select * from test;
+ x | is_cycle |                     path                      
+---+----------+-----------------------------------------------
+ 0 | f        | {(0)}
+ 1 | f        | {(0),(1)}
+ 2 | f        | {(0),(1),(2)}
+ 3 | f        | {(0),(1),(2),(3)}
+ 4 | f        | {(0),(1),(2),(3),(4)}
+ 5 | f        | {(0),(1),(2),(3),(4),(5)}
+ 6 | f        | {(0),(1),(2),(3),(4),(5),(6)}
+ 7 | f        | {(0),(1),(2),(3),(4),(5),(6),(7)}
+ 8 | f        | {(0),(1),(2),(3),(4),(5),(6),(7),(8)}
+ 9 | f        | {(0),(1),(2),(3),(4),(5),(6),(7),(8),(9)}
+ 0 | t        | {(0),(1),(2),(3),(4),(5),(6),(7),(8),(9),(0)}
+(11 rows)
+
 -- multiple CTEs
 with recursive
 graph(f, t, label) as (

diff --git a/src/test/regress/sql/with.sql b/src/test/regress/sql/with.sql
index def3587d34a049c4cecf62687bea86b61bf39852..dc36b7ccb04e0130b04589b50a5037c50ce0bfe7 100644 (file)
--- a/src/test/regress/sql/with.sql
+++ b/src/test/regress/sql/with.sql
@@ -605,6 +605,15 @@ with recursive test as (
 ) cycle x set is_cycle using path
 select * from test;
 
+with recursive test as (
+  select 0 as x
+  union all
+  select (x + 1) % 10
+  from test
+    where not is_cycle  -- redundant, but legal
+) cycle x set is_cycle using path
+select * from test;
+
 -- multiple CTEs
 with recursive
 graph(f, t, label) as (