From: Tom Lane <tgl@sss.pgh.pa.us>
To: exclusion@gmail.com
Cc: pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #18536: Using WITH inside WITH RECURSIVE triggers a "shouldn't happen" error
Date: Sun, 14 Jul 2024 13:01:32 -0400
Message-ID: <2866155.1720976492@sss.pgh.pa.us> (raw)
In-Reply-To: <2781444.1720969777@sss.pgh.pa.us>
References: <18536-0a342ec07901203e@postgresql.org>
<2781444.1720969777@sss.pgh.pa.us>
I wrote:
> I think the case you show should be throwing
> ERROR: recursive query "t" does not have the form non-recursive-term UNION [ALL] recursive-term
Hmm, that is probably too strong: it will break some queries we've
historically accepted. What we need is just to forbid self-references
within the WITH clause. The code actually does that already, it's
just doing it too late; so we can fix this with a simple re-ordering
of the error checks, as attached.
regards, tom lane
Attachments:fix-bug-18536.patch (text/x-diff, 4.0K) downloaddiff --git a/src/backend/parser/parse_cte.c b/src/backend/parser/parse_cte.cindex 6826d4f36a..17432e9df6 100644--- a/src/backend/parser/parse_cte.c+++ b/src/backend/parser/parse_cte.c@@ -877,6 +877,25 @@ checkWellFormedRecursion(CteState *cstate)
cte->ctename),
parser_errposition(cstate->pstate, cte->location)));
+ /*+ * Really, we should insist that there not be a top-level WITH, since+ * syntactically that would enclose the UNION. However, we've not+ * done so in the past and it's probably too late to change. Settle+ * for insisting that WITH not contain a self-reference. Test this+ * before examining the UNION arms, to avoid issuing confusing errors+ * in such cases.+ */+ if (stmt->withClause)+ {+ cstate->curitem = i;+ cstate->innerwiths = NIL;+ cstate->selfrefcount = 0;+ cstate->context = RECURSION_SUBLINK;+ checkWellFormedRecursionWalker((Node *) stmt->withClause->ctes,+ cstate);+ Assert(cstate->innerwiths == NIL);+ }+
/* The left-hand operand mustn't contain self-reference at all */
cstate->curitem = i;
cstate->innerwiths = NIL;
@@ -895,18 +914,6 @@ checkWellFormedRecursion(CteState *cstate)
if (cstate->selfrefcount != 1) /* shouldn't happen */
elog(ERROR, "missing recursive reference");
- /* WITH mustn't contain self-reference, either */- if (stmt->withClause)- {- cstate->curitem = i;- cstate->innerwiths = NIL;- cstate->selfrefcount = 0;- cstate->context = RECURSION_SUBLINK;- checkWellFormedRecursionWalker((Node *) stmt->withClause->ctes,- cstate);- Assert(cstate->innerwiths == NIL);- }-
/*
* Disallow ORDER BY and similar decoration atop the UNION. These
* don't make sense because it's impossible to figure out what they
diff --git a/src/test/regress/expected/with.out b/src/test/regress/expected/with.outindex b4f3121751..fff2d4949e 100644--- a/src/test/regress/expected/with.out+++ b/src/test/regress/expected/with.out@@ -2029,6 +2029,38 @@ WITH RECURSIVE x(n) AS (SELECT n FROM x UNION ALL SELECT 1)
ERROR: recursive reference to query "x" must not appear within its non-recursive term
LINE 1: WITH RECURSIVE x(n) AS (SELECT n FROM x UNION ALL SELECT 1)
^
+-- allow this, because we historically have+WITH RECURSIVE x(n) AS (+ WITH x1 AS (SELECT 1 AS n)+ SELECT 0+ UNION+ SELECT * FROM x1)+ SELECT * FROM x;+ n +---+ 0+ 1+(2 rows)++-- but this should be rejected+WITH RECURSIVE x(n) AS (+ WITH x1 AS (SELECT 1 FROM x)+ SELECT 0+ UNION+ SELECT * FROM x1)+ SELECT * FROM x;+ERROR: recursive reference to query "x" must not appear within a subquery+LINE 2: WITH x1 AS (SELECT 1 FROM x)+ ^+-- and this too+WITH RECURSIVE x(n) AS (+ (WITH x1 AS (SELECT 1 FROM x) SELECT * FROM x1)+ UNION+ SELECT 0)+ SELECT * FROM x;+ERROR: recursive reference to query "x" must not appear within its non-recursive term+LINE 2: (WITH x1 AS (SELECT 1 FROM x) SELECT * FROM x1)+ ^
CREATE TEMPORARY TABLE y (a INTEGER);
INSERT INTO y SELECT generate_series(1, 10);
-- LEFT JOIN
diff --git a/src/test/regress/sql/with.sql b/src/test/regress/sql/with.sqlindex 3fb0b33fce..06b2d4857f 100644--- a/src/test/regress/sql/with.sql+++ b/src/test/regress/sql/with.sql@@ -908,6 +908,29 @@ WITH RECURSIVE x(n) AS (SELECT n FROM x)
WITH RECURSIVE x(n) AS (SELECT n FROM x UNION ALL SELECT 1)
SELECT * FROM x;
+-- allow this, because we historically have+WITH RECURSIVE x(n) AS (+ WITH x1 AS (SELECT 1 AS n)+ SELECT 0+ UNION+ SELECT * FROM x1)+ SELECT * FROM x;++-- but this should be rejected+WITH RECURSIVE x(n) AS (+ WITH x1 AS (SELECT 1 FROM x)+ SELECT 0+ UNION+ SELECT * FROM x1)+ SELECT * FROM x;++-- and this too+WITH RECURSIVE x(n) AS (+ (WITH x1 AS (SELECT 1 FROM x) SELECT * FROM x1)+ UNION+ SELECT 0)+ SELECT * FROM x;+
CREATE TEMPORARY TABLE y (a INTEGER);
INSERT INTO y SELECT generate_series(1, 10);
=
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, exclusion@gmail.com, pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #18536: Using WITH inside WITH RECURSIVE triggers a "shouldn't happen" error
In-Reply-To: <2866155.1720976492@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