Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

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

 Sign up
Appearance settings

Commite276b58

Browse files
committed
Fix parse_cte.c's failure to examine sub-WITHs in DML statements.
makeDependencyGraphWalker thought that only SelectStmt nodes couldcontain a WithClause. Which was true in our original implementationof WITH, but astonishingly we missed updating this code when we addedthe ability to attach WITH to INSERT/UPDATE/DELETE (and later MERGE).Moreover, since it was coded to deliberately block recursion to aWithClause, even updating raw_expression_tree_walker didn't save it.The upshot of this was that we didn't see references to outer CTEnames appearing within an inner WITH, and would neither complain aboutdisallowed recursion nor account for such references when sorting CTEsinto a usable order. The lack of complaints about this is perhaps notso surprising, because typical usage of WITH wouldn't hit either case.Still, it's pretty broken; failing to detect recursion here leads toassert failures or worse later on.Fix by factoring out the processing of sub-WITHs into a new functionWalkInnerWith, and invoking that for all the statement types thatcan have WITH.Bug: #18878Reported-by: Yu Liang <luy70@psu.edu>Author: Tom Lane <tgl@sss.pgh.pa.us>Discussion:https://postgr.es/m/18878-a26fa5ab6be2f2cf@postgresql.orgBackpatch-through: 13
1 parentb92482d commite276b58

File tree

3 files changed

+111
-43
lines changed

3 files changed

+111
-43
lines changed

‎src/backend/parser/parse_cte.c

Lines changed: 96 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ static void analyzeCTE(ParseState *pstate, CommonTableExpr *cte);
8484
/* Dependency processing functions */
8585
staticvoidmakeDependencyGraph(CteState*cstate);
8686
staticboolmakeDependencyGraphWalker(Node*node,CteState*cstate);
87+
staticvoidWalkInnerWith(Node*stmt,WithClause*withClause,CteState*cstate);
8788
staticvoidTopologicalSort(ParseState*pstate,CteItem*items,intnumitems);
8889

8990
/* Recursion validity checker functions */
@@ -507,58 +508,56 @@ makeDependencyGraphWalker(Node *node, CteState *cstate)
507508
if (IsA(node,SelectStmt))
508509
{
509510
SelectStmt*stmt= (SelectStmt*)node;
510-
ListCell*lc;
511511

512512
if (stmt->withClause)
513513
{
514-
if (stmt->withClause->recursive)
515-
{
516-
/*
517-
* In the RECURSIVE case, all query names of the WITH are
518-
* visible to all WITH items as well as the main query. So
519-
* push them all on, process, pop them all off.
520-
*/
521-
cstate->innerwiths=lcons(stmt->withClause->ctes,
522-
cstate->innerwiths);
523-
foreach(lc,stmt->withClause->ctes)
524-
{
525-
CommonTableExpr*cte= (CommonTableExpr*)lfirst(lc);
514+
/* Examine the WITH clause and the SelectStmt */
515+
WalkInnerWith(node,stmt->withClause,cstate);
516+
/* We're done examining the SelectStmt */
517+
return false;
518+
}
519+
/* if no WITH clause, just fall through for normal processing */
520+
}
521+
elseif (IsA(node,InsertStmt))
522+
{
523+
InsertStmt*stmt= (InsertStmt*)node;
526524

527-
(void)makeDependencyGraphWalker(cte->ctequery,cstate);
528-
}
529-
(void)raw_expression_tree_walker(node,
530-
makeDependencyGraphWalker,
531-
(void*)cstate);
532-
cstate->innerwiths=list_delete_first(cstate->innerwiths);
533-
}
534-
else
535-
{
536-
/*
537-
* In the non-RECURSIVE case, query names are visible to the
538-
* WITH items after them and to the main query.
539-
*/
540-
cstate->innerwiths=lcons(NIL,cstate->innerwiths);
541-
foreach(lc,stmt->withClause->ctes)
542-
{
543-
CommonTableExpr*cte= (CommonTableExpr*)lfirst(lc);
544-
ListCell*cell1;
525+
if (stmt->withClause)
526+
{
527+
/* Examine the WITH clause and the InsertStmt */
528+
WalkInnerWith(node,stmt->withClause,cstate);
529+
/* We're done examining the InsertStmt */
530+
return false;
531+
}
532+
/* if no WITH clause, just fall through for normal processing */
533+
}
534+
elseif (IsA(node,DeleteStmt))
535+
{
536+
DeleteStmt*stmt= (DeleteStmt*)node;
545537

546-
(void)makeDependencyGraphWalker(cte->ctequery,cstate);
547-
/* note that recursion could mutate innerwiths list */
548-
cell1=list_head(cstate->innerwiths);
549-
lfirst(cell1)=lappend((List*)lfirst(cell1),cte);
550-
}
551-
(void)raw_expression_tree_walker(node,
552-
makeDependencyGraphWalker,
553-
(void*)cstate);
554-
cstate->innerwiths=list_delete_first(cstate->innerwiths);
555-
}
556-
/* We're done examining the SelectStmt */
538+
if (stmt->withClause)
539+
{
540+
/* Examine the WITH clause and the DeleteStmt */
541+
WalkInnerWith(node,stmt->withClause,cstate);
542+
/* We're done examining the DeleteStmt */
557543
return false;
558544
}
559545
/* if no WITH clause, just fall through for normal processing */
560546
}
561-
if (IsA(node,WithClause))
547+
elseif (IsA(node,UpdateStmt))
548+
{
549+
UpdateStmt*stmt= (UpdateStmt*)node;
550+
551+
if (stmt->withClause)
552+
{
553+
/* Examine the WITH clause and the UpdateStmt */
554+
WalkInnerWith(node,stmt->withClause,cstate);
555+
/* We're done examining the UpdateStmt */
556+
return false;
557+
}
558+
/* if no WITH clause, just fall through for normal processing */
559+
}
560+
elseif (IsA(node,WithClause))
562561
{
563562
/*
564563
* Prevent raw_expression_tree_walker from recursing directly into a
@@ -572,6 +571,60 @@ makeDependencyGraphWalker(Node *node, CteState *cstate)
572571
(void*)cstate);
573572
}
574573

574+
/*
575+
* makeDependencyGraphWalker's recursion into a statement having a WITH clause.
576+
*
577+
* This subroutine is concerned with updating the innerwiths list correctly
578+
* based on the visibility rules for CTE names.
579+
*/
580+
staticvoid
581+
WalkInnerWith(Node*stmt,WithClause*withClause,CteState*cstate)
582+
{
583+
ListCell*lc;
584+
585+
if (withClause->recursive)
586+
{
587+
/*
588+
* In the RECURSIVE case, all query names of the WITH are visible to
589+
* all WITH items as well as the main query. So push them all on,
590+
* process, pop them all off.
591+
*/
592+
cstate->innerwiths=lcons(withClause->ctes,cstate->innerwiths);
593+
foreach(lc,withClause->ctes)
594+
{
595+
CommonTableExpr*cte= (CommonTableExpr*)lfirst(lc);
596+
597+
(void)makeDependencyGraphWalker(cte->ctequery,cstate);
598+
}
599+
(void)raw_expression_tree_walker(stmt,
600+
makeDependencyGraphWalker,
601+
(void*)cstate);
602+
cstate->innerwiths=list_delete_first(cstate->innerwiths);
603+
}
604+
else
605+
{
606+
/*
607+
* In the non-RECURSIVE case, query names are visible to the WITH
608+
* items after them and to the main query.
609+
*/
610+
cstate->innerwiths=lcons(NIL,cstate->innerwiths);
611+
foreach(lc,withClause->ctes)
612+
{
613+
CommonTableExpr*cte= (CommonTableExpr*)lfirst(lc);
614+
ListCell*cell1;
615+
616+
(void)makeDependencyGraphWalker(cte->ctequery,cstate);
617+
/* note that recursion could mutate innerwiths list */
618+
cell1=list_head(cstate->innerwiths);
619+
lfirst(cell1)=lappend((List*)lfirst(cell1),cte);
620+
}
621+
(void)raw_expression_tree_walker(stmt,
622+
makeDependencyGraphWalker,
623+
(void*)cstate);
624+
cstate->innerwiths=list_delete_first(cstate->innerwiths);
625+
}
626+
}
627+
575628
/*
576629
* Sort by dependencies, using a standard topological sort operation
577630
*/

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1109,6 +1109,14 @@ WITH RECURSIVE x(n) AS (
11091109
ERROR: ORDER BY in a recursive query is not implemented
11101110
LINE 3: ORDER BY (SELECT n FROM x))
11111111
^
1112+
-- and this
1113+
WITH RECURSIVE x(n) AS (
1114+
WITH sub_cte AS (SELECT * FROM x)
1115+
DELETE FROM graph RETURNING f)
1116+
SELECT * FROM x;
1117+
ERROR: recursive query "x" must not contain data-modifying statements
1118+
LINE 1: WITH RECURSIVE x(n) AS (
1119+
^
11121120
CREATE TEMPORARY TABLE y (a INTEGER);
11131121
INSERT INTO y SELECT generate_series(1, 10);
11141122
-- LEFT JOIN

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -520,6 +520,13 @@ WITH RECURSIVE x(n) AS (
520520
ORDER BY (SELECT nFROM x))
521521
SELECT*FROM x;
522522

523+
-- and this
524+
WITH RECURSIVE x(n)AS (
525+
WITH sub_cteAS (SELECT*FROM x)
526+
DELETEFROM graph RETURNING f)
527+
SELECT*FROM x;
528+
529+
523530
CREATE TEMPORARY TABLE y (aINTEGER);
524531
INSERT INTO ySELECT generate_series(1,10);
525532

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp