diff options
author | Tom Lane <tgl@sss.pgh.pa.us> | 2022-04-23 12:16:12 -0400 |
---|---|---|
committer | Tom Lane <tgl@sss.pgh.pa.us> | 2022-04-23 12:16:12 -0400 |
commit | f819020d400f1dbd96ee1a9fd44a1f6f44932b4d (patch) | |
tree | 312fb630148146855cd0659e1e55f0113f9c8f57 /src | |
parent | c1da0acbb06e9175044b436d14c51cef03339109 (diff) | |
download | postgresql-f819020d400f1dbd96ee1a9fd44a1f6f44932b4d.tar.gz postgresql-f819020d400f1dbd96ee1a9fd44a1f6f44932b4d.zip |
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
Diffstat (limited to 'src')
-rw-r--r-- | src/backend/rewrite/rewriteSearchCycle.c | 19 | ||||
-rw-r--r-- | src/test/regress/expected/with.out | 10 | ||||
-rw-r--r-- | src/test/regress/sql/with.sql | 10 |
3 files changed, 36 insertions, 3 deletions
diff --git a/src/backend/rewrite/rewriteSearchCycle.c b/src/backend/rewrite/rewriteSearchCycle.c index dc408404ebe..58f684cd52e 100644 --- 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 32f90d73750..d7316043748 100644 --- 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 7e430e859ea..3251c29584b 100644 --- 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 ( |