Fix incautious CTE matching in rewriteSearchAndCycle().

This function looks for a reference to the recursive WITH CTE,
but it checked only the CTE name not ctelevelsup, so that it could
seize on a lower CTE that happened to have the same name.  This
would result in planner failures later, either weird errors such as
"could not find attribute 2 in subquery targetlist", or crashes
or assertion failures.  The code also merely Assert'ed that it found
a matching entry, which is not guaranteed at all by the parser.

Per bugs #17320 and #17318 from Zhiyong Wu.
Thanks to Kyotaro Horiguchi for investigation.

Discussion: https://postgr.es/m/17320-70e37868182512ab@postgresql.org
Discussion: https://postgr.es/m/17318-2eb65a3a611d2368@postgresql.org

diff --git a/src/backend/rewrite/rewriteSearchCycle.c b/src/backend/rewrite/rewriteSearchCycle.c
index dc408404ebe18b56c351b8f81ca30a924755d529..58f684cd52e2d069c95c0486a303c51149b3bf0c 100644 (file)
--- a/src/backend/rewrite/rewriteSearchCycle.c
+++ b/src/backend/rewrite/rewriteSearchCycle.c
@@ -383,19 +383,32 @@ rewriteSearchAndCycle(CommonTableExpr *cte)
        newrte->eref = newrte->alias;
 
        /*
-        * Find the reference to our CTE in the range table
+        * Find the reference to the recursive CTE in the right UNION subquery's
+        * range table.  We expect it to be two levels up from the UNION subquery
+        * (and must check that to avoid being fooled by sub-WITHs with the same
+        * CTE name).  There will not be more than one such reference, because the
+        * parser would have rejected that (see checkWellFormedRecursion() in
+        * parse_cte.c).  However, the parser doesn't insist that the reference
+        * appear in the UNION subquery's topmost range table, so we might fail to
+        * find it at all.  That's an unimplemented case for the moment.
         */
        for (int rti = 1; rti <= list_length(rte2->subquery->rtable); rti++)
        {
                RangeTblEntry *e = rt_fetch(rti, rte2->subquery->rtable);
 
-               if (e->rtekind == RTE_CTE && strcmp(cte->ctename, e->ctename) == 0)
+               if (e->rtekind == RTE_CTE &&
+                       strcmp(cte->ctename, e->ctename) == 0 &&
+                       e->ctelevelsup == 2)
                {
                        cte_rtindex = rti;
                        break;
                }
        }
-       Assert(cte_rtindex > 0);
+       if (cte_rtindex <= 0)
+               ereport(ERROR,
+                               (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+                                errmsg("with a SEARCH or CYCLE clause, the recursive reference to WITH query \"%s\" must be at the top level of its right-hand SELECT",
+                                               cte->ctename)));
 
        newsubquery = copyObject(rte2->subquery);
        IncrementVarSublevelsUp((Node *) newsubquery, 1, 1);

diff --git a/src/test/regress/expected/with.out b/src/test/regress/expected/with.out
index 32f90d73750a037e7b5e8b24328ebdb6bb85a4ca..d7316043748f230bc898fae80f7f5e17ce0bb935 100644 (file)
--- a/src/test/regress/expected/with.out
+++ b/src/test/regress/expected/with.out
@@ -846,6 +846,16 @@ with recursive search_graph(f, t, label) as (
 ) search depth first by f, t set seq
 select * from search_graph order by seq;
 ERROR:  with a SEARCH or CYCLE clause, the right side of the UNION must be a SELECT
+-- check that we distinguish same CTE name used at different levels
+-- (this case could be supported, perhaps, but it isn't today)
+with recursive x(col) as (
+       select 1
+       union
+       (with x as (select * from x)
+        select * from x)
+) search depth first by col set seq
+select * from x;
+ERROR:  with a SEARCH or CYCLE clause, the recursive reference to WITH query "x" must be at the top level of its right-hand SELECT
 -- test ruleutils and view expansion
 create temp view v_search as
 with recursive search_graph(f, t, label) as (

diff --git a/src/test/regress/sql/with.sql b/src/test/regress/sql/with.sql
index 7e430e859eacb9c26131dbac035880d05b5c5946..3251c29584bd8bcfefc007b8f851e4f9aa9af735 100644 (file)
--- a/src/test/regress/sql/with.sql
+++ b/src/test/regress/sql/with.sql
@@ -464,6 +464,16 @@ with recursive search_graph(f, t, label) as (
 ) search depth first by f, t set seq
 select * from search_graph order by seq;
 
+-- check that we distinguish same CTE name used at different levels
+-- (this case could be supported, perhaps, but it isn't today)
+with recursive x(col) as (
+       select 1
+       union
+       (with x as (select * from x)
+        select * from x)
+) search depth first by col set seq
+select * from x;
+
 -- test ruleutils and view expansion
 create temp view v_search as
 with recursive search_graph(f, t, label) as (