Skip to content

Navigation Menu

Sign in

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit f96c2c7

Browse filesBrowse files
tglsfdctglsfdc
committed
Avoid unhelpful internal error for incorrect recursive-WITH queries.
checkWellFormedRecursion would issue "missing recursive reference" if a WITH RECURSIVE query contained a single self-reference but that self-reference was inside a top-level WITH, ORDER BY, LIMIT, etc, rather than inside the second arm of the UNION as expected. We already intended to throw more-on-point errors for such cases, but those error checks must be done before examining the UNION arm in order to have the desired results. So this patch need only move some code (and improve the comments). Per bug #18536 from Alexander Lakhin. Back-patch to all supported branches. Discussion: https://postgr.es/m/18536-0a342ec07901203e@postgresql.org
1 parent d5e6891 commit f96c2c7
Copy full SHA for f96c2c7

File tree

3 files changed

+102
-20
lines changed
Filter options

3 files changed

+102
-20
lines changed

‎src/backend/parser/parse_cte.c

Copy file name to clipboardExpand all lines: src/backend/parser/parse_cte.c
+33-20Lines changed: 33 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -877,25 +877,14 @@ checkWellFormedRecursion(CteState *cstate)
877877
cte->ctename),
878878
parser_errposition(cstate->pstate, cte->location)));
879879

880-
/* The left-hand operand mustn't contain self-reference at all */
881-
cstate->curitem = i;
882-
cstate->innerwiths = NIL;
883-
cstate->selfrefcount = 0;
884-
cstate->context = RECURSION_NONRECURSIVETERM;
885-
checkWellFormedRecursionWalker((Node *) stmt->larg, cstate);
886-
Assert(cstate->innerwiths == NIL);
887-
888-
/* Right-hand operand should contain one reference in a valid place */
889-
cstate->curitem = i;
890-
cstate->innerwiths = NIL;
891-
cstate->selfrefcount = 0;
892-
cstate->context = RECURSION_OK;
893-
checkWellFormedRecursionWalker((Node *) stmt->rarg, cstate);
894-
Assert(cstate->innerwiths == NIL);
895-
if (cstate->selfrefcount != 1) /* shouldn't happen */
896-
elog(ERROR, "missing recursive reference");
897-
898-
/* WITH mustn't contain self-reference, either */
880+
/*
881+
* Really, we should insist that there not be a top-level WITH, since
882+
* syntactically that would enclose the UNION. However, we've not
883+
* done so in the past and it's probably too late to change. Settle
884+
* for insisting that WITH not contain a self-reference. Test this
885+
* before examining the UNION arms, to avoid issuing confusing errors
886+
* in such cases.
887+
*/
899888
if (stmt->withClause)
900889
{
901890
cstate->curitem = i;
@@ -912,7 +901,9 @@ checkWellFormedRecursion(CteState *cstate)
912901
* don't make sense because it's impossible to figure out what they
913902
* mean when we have only part of the recursive query's results. (If
914903
* we did allow them, we'd have to check for recursive references
915-
* inside these subtrees.)
904+
* inside these subtrees. As for WITH, we have to do this before
905+
* examining the UNION arms, to avoid issuing confusing errors if
906+
* there is a recursive reference here.)
916907
*/
917908
if (stmt->sortClause)
918909
ereport(ERROR,
@@ -938,6 +929,28 @@ checkWellFormedRecursion(CteState *cstate)
938929
errmsg("FOR UPDATE/SHARE in a recursive query is not implemented"),
939930
parser_errposition(cstate->pstate,
940931
exprLocation((Node *) stmt->lockingClause))));
932+
933+
/*
934+
* Now we can get on with checking the UNION operands themselves.
935+
*
936+
* The left-hand operand mustn't contain a self-reference at all.
937+
*/
938+
cstate->curitem = i;
939+
cstate->innerwiths = NIL;
940+
cstate->selfrefcount = 0;
941+
cstate->context = RECURSION_NONRECURSIVETERM;
942+
checkWellFormedRecursionWalker((Node *) stmt->larg, cstate);
943+
Assert(cstate->innerwiths == NIL);
944+
945+
/* Right-hand operand should contain one reference in a valid place */
946+
cstate->curitem = i;
947+
cstate->innerwiths = NIL;
948+
cstate->selfrefcount = 0;
949+
cstate->context = RECURSION_OK;
950+
checkWellFormedRecursionWalker((Node *) stmt->rarg, cstate);
951+
Assert(cstate->innerwiths == NIL);
952+
if (cstate->selfrefcount != 1) /* shouldn't happen */
953+
elog(ERROR, "missing recursive reference");
941954
}
942955
}
943956

‎src/test/regress/expected/with.out

Copy file name to clipboardExpand all lines: src/test/regress/expected/with.out
+40Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2029,6 +2029,46 @@ WITH RECURSIVE x(n) AS (SELECT n FROM x UNION ALL SELECT 1)
20292029
ERROR: recursive reference to query "x" must not appear within its non-recursive term
20302030
LINE 1: WITH RECURSIVE x(n) AS (SELECT n FROM x UNION ALL SELECT 1)
20312031
^
2032+
-- allow this, because we historically have
2033+
WITH RECURSIVE x(n) AS (
2034+
WITH x1 AS (SELECT 1 AS n)
2035+
SELECT 0
2036+
UNION
2037+
SELECT * FROM x1)
2038+
SELECT * FROM x;
2039+
n
2040+
---
2041+
0
2042+
1
2043+
(2 rows)
2044+
2045+
-- but this should be rejected
2046+
WITH RECURSIVE x(n) AS (
2047+
WITH x1 AS (SELECT 1 FROM x)
2048+
SELECT 0
2049+
UNION
2050+
SELECT * FROM x1)
2051+
SELECT * FROM x;
2052+
ERROR: recursive reference to query "x" must not appear within a subquery
2053+
LINE 2: WITH x1 AS (SELECT 1 FROM x)
2054+
^
2055+
-- and this too
2056+
WITH RECURSIVE x(n) AS (
2057+
(WITH x1 AS (SELECT 1 FROM x) SELECT * FROM x1)
2058+
UNION
2059+
SELECT 0)
2060+
SELECT * FROM x;
2061+
ERROR: recursive reference to query "x" must not appear within its non-recursive term
2062+
LINE 2: (WITH x1 AS (SELECT 1 FROM x) SELECT * FROM x1)
2063+
^
2064+
-- and this
2065+
WITH RECURSIVE x(n) AS (
2066+
SELECT 0 UNION SELECT 1
2067+
ORDER BY (SELECT n FROM x))
2068+
SELECT * FROM x;
2069+
ERROR: ORDER BY in a recursive query is not implemented
2070+
LINE 3: ORDER BY (SELECT n FROM x))
2071+
^
20322072
CREATE TEMPORARY TABLE y (a INTEGER);
20332073
INSERT INTO y SELECT generate_series(1, 10);
20342074
-- LEFT JOIN

‎src/test/regress/sql/with.sql

Copy file name to clipboardExpand all lines: src/test/regress/sql/with.sql
+29Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -908,6 +908,35 @@ WITH RECURSIVE x(n) AS (SELECT n FROM x)
908908
WITH RECURSIVE x(n) AS (SELECT n FROM x UNION ALL SELECT 1)
909909
SELECT * FROM x;
910910

911+
-- allow this, because we historically have
912+
WITH RECURSIVE x(n) AS (
913+
WITH x1 AS (SELECT 1 AS n)
914+
SELECT 0
915+
UNION
916+
SELECT * FROM x1)
917+
SELECT * FROM x;
918+
919+
-- but this should be rejected
920+
WITH RECURSIVE x(n) AS (
921+
WITH x1 AS (SELECT 1 FROM x)
922+
SELECT 0
923+
UNION
924+
SELECT * FROM x1)
925+
SELECT * FROM x;
926+
927+
-- and this too
928+
WITH RECURSIVE x(n) AS (
929+
(WITH x1 AS (SELECT 1 FROM x) SELECT * FROM x1)
930+
UNION
931+
SELECT 0)
932+
SELECT * FROM x;
933+
934+
-- and this
935+
WITH RECURSIVE x(n) AS (
936+
SELECT 0 UNION SELECT 1
937+
ORDER BY (SELECT n FROM x))
938+
SELECT * FROM x;
939+
911940
CREATE TEMPORARY TABLE y (a INTEGER);
912941
INSERT INTO y SELECT generate_series(1, 10);
913942

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.