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 8fc86586085ef28b3c13c1e5650d09eba814e2e7..bf2d9d8287432446fd2a788a104a6588bd4dd2cd 100644 (file)
--- a/src/backend/parser/parse_cte.c
+++ b/src/backend/parser/parse_cte.c
@@ -248,10 +248,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;
 
@@ -346,7 +412,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;
@@ -393,12 +462,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)
        {
            String     *colname = lfirst_node(String, lc);
 
@@ -407,33 +476,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)
        {
            String     *colname = lfirst_node(String, lc);
 
@@ -442,97 +509,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 f3fd1cd32adba46b77f4f27a0e17300af67af381..30f2574c8814f6209e75723c4bf46ff8c06b67e2 100644 (file)
--- a/src/test/regress/expected/with.out
+++ b/src/test/regress/expected/with.out
@@ -1252,6 +1252,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 f876c663519033e813401ed5f2f0ab6f439bfe0e..2148972ea2338bed283dad78f8c890dc6ff755e6 100644 (file)
--- a/src/test/regress/sql/with.sql
+++ b/src/test/regress/sql/with.sql
@@ -613,6 +613,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 (