From: Tom Lane <tgl@sss.pgh.pa.us>
To: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Cc: 253540651@qq.com, pgsql-bugs@lists.postgresql.org, Peter Eisentraut <peter.eisentraut@enterprisedb.com>
Subject: Re: BUG #17320: A SEGV in optimizer
Date: Fri, 22 Apr 2022 17:43:43 -0400
Message-ID: <3774893.1650663823@sss.pgh.pa.us> (raw)
In-Reply-To: <20211207.172546.1329872704555357126.horikyota.ntt@gmail.com>
References: <17320-70e37868182512ab@postgresql.org>
<20211207.172546.1329872704555357126.horikyota.ntt@gmail.com>
[ this'd been on my to-do list for a long time, I finally got to it ]
Kyotaro Horiguchi <horikyota.ntt@gmail.com> writes:
> If I tried to execute the following query, I got the follwoing
> error. This looks like rooted from the same mistake.
> WITH RECURSIVE x ( x ) AS
> (SELECT 1
> UNION
> (WITH x AS (SELECT * FROM x)
> SELECT * FROM x)
> ) SEARCH BREADTH FIRST BY x SET NCHAR
> SELECT * FROM x;
> ERROR: could not find attribute 2 in subquery targetlist
> By the way, if I executed the following query, I get the following
> another assertion failure.
> WITH RECURSIVE x ( x ) AS
> (SELECT 1
> UNION
> (WITH y AS (SELECT * FROM x)
> SELECT * FROM y)
> ) SEARCH BREADTH FIRST BY x SET NCHAR
> SELECT * FROM x;
> TRAP: FailedAssertion("cte_rtindex > 0", File: "rewriteSearchCycle.c", Line: 398, PID: 31288)
This is pretty remarkable, because those two queries *should* be
semantically identical. I guessed that something is getting confused
by the duplicated CTE name, and it didn't take long to see what:
rewriteSearchAndCycle() is failing to check ctelevelsup while looking
for a match to the recursive CTE's name. Thus, it thinks that the
reference to the lower WITH item (either "x" or "y" in these two
examples) is the recursive reference it seeks, which it ain't.
Eventually perhaps we should support this, since a recursive query
of this form is allowed in other contexts. But I'm content to
throw error for now, as per the attached patch. This catches all
the cases shown in this thread, as well as the seemingly unrelated
error shown in bug #17318.
(I looked for other places that might be carelessly matching on
ctename without checking levelsup, and found none.)
regards, tom lane
Attachments:
[text/x-diff] detect-incorrect-recursive-ref-in-SEARCH-and-CYCLE.patch (3.3K, ../3774893.1650663823@sss.pgh.pa.us/2-detect-incorrect-recursive-ref-in-SEARCH-and-CYCLE.patch)
download | inline diff:diff --git a/src/backend/rewrite/rewriteSearchCycle.c b/src/backend/rewrite/rewriteSearchCycle.cindex dc408404eb..58f684cd52 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.outindex 32f90d7375..d731604374 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.sqlindex 7e430e859e..3251c29584 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 (
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: tgl@sss.pgh.pa.us, horikyota.ntt@gmail.com, peter.eisentraut@enterprisedb.com
Subject: Re: BUG #17320: A SEGV in optimizer
In-Reply-To: <3774893.1650663823@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox