Re: BUG #18536: Using WITH inside WITH RECURSIVE triggers a "shouldn't happen" error - Mailing list pgsql-bugs
From | Tom Lane |
---|---|
Subject | Re: BUG #18536: Using WITH inside WITH RECURSIVE triggers a "shouldn't happen" error |
Date | |
Msg-id | 2866155.1720976492@sss.pgh.pa.us Whole thread Raw |
In response to | Re: BUG #18536: Using WITH inside WITH RECURSIVE triggers a "shouldn't happen" error (Tom Lane <tgl@sss.pgh.pa.us>) |
Responses |
Re: BUG #18536: Using WITH inside WITH RECURSIVE triggers a "shouldn't happen" error
|
List | pgsql-bugs |
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 diff --git a/src/backend/parser/parse_cte.c b/src/backend/parser/parse_cte.c index 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.out index 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.sql index 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);
pgsql-bugs by date: