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

Commit5e7be43

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 parent717e8a1 commit5e7be43

File tree

3 files changed

+124
-43
lines changed

3 files changed

+124
-43
lines changed

‎src/backend/parser/parse_cte.c

Lines changed: 109 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ static void analyzeCTE(ParseState *pstate, CommonTableExpr *cte);
8888
/* Dependency processing functions */
8989
staticvoidmakeDependencyGraph(CteState*cstate);
9090
staticboolmakeDependencyGraphWalker(Node*node,CteState*cstate);
91+
staticvoidWalkInnerWith(Node*stmt,WithClause*withClause,CteState*cstate);
9192
staticvoidTopologicalSort(ParseState*pstate,CteItem*items,intnumitems);
9293

9394
/* Recursion validity checker functions */
@@ -725,58 +726,69 @@ makeDependencyGraphWalker(Node *node, CteState *cstate)
725726
if (IsA(node,SelectStmt))
726727
{
727728
SelectStmt*stmt= (SelectStmt*)node;
728-
ListCell*lc;
729729

730730
if (stmt->withClause)
731731
{
732-
if (stmt->withClause->recursive)
733-
{
734-
/*
735-
* In the RECURSIVE case, all query names of the WITH are
736-
* visible to all WITH items as well as the main query. So
737-
* push them all on, process, pop them all off.
738-
*/
739-
cstate->innerwiths=lcons(stmt->withClause->ctes,
740-
cstate->innerwiths);
741-
foreach(lc,stmt->withClause->ctes)
742-
{
743-
CommonTableExpr*cte= (CommonTableExpr*)lfirst(lc);
732+
/* Examine the WITH clause and the SelectStmt */
733+
WalkInnerWith(node,stmt->withClause,cstate);
734+
/* We're done examining the SelectStmt */
735+
return false;
736+
}
737+
/* if no WITH clause, just fall through for normal processing */
738+
}
739+
elseif (IsA(node,InsertStmt))
740+
{
741+
InsertStmt*stmt= (InsertStmt*)node;
744742

745-
(void)makeDependencyGraphWalker(cte->ctequery,cstate);
746-
}
747-
(void)raw_expression_tree_walker(node,
748-
makeDependencyGraphWalker,
749-
(void*)cstate);
750-
cstate->innerwiths=list_delete_first(cstate->innerwiths);
751-
}
752-
else
753-
{
754-
/*
755-
* In the non-RECURSIVE case, query names are visible to the
756-
* WITH items after them and to the main query.
757-
*/
758-
cstate->innerwiths=lcons(NIL,cstate->innerwiths);
759-
foreach(lc,stmt->withClause->ctes)
760-
{
761-
CommonTableExpr*cte= (CommonTableExpr*)lfirst(lc);
762-
ListCell*cell1;
743+
if (stmt->withClause)
744+
{
745+
/* Examine the WITH clause and the InsertStmt */
746+
WalkInnerWith(node,stmt->withClause,cstate);
747+
/* We're done examining the InsertStmt */
748+
return false;
749+
}
750+
/* if no WITH clause, just fall through for normal processing */
751+
}
752+
elseif (IsA(node,DeleteStmt))
753+
{
754+
DeleteStmt*stmt= (DeleteStmt*)node;
763755

764-
(void)makeDependencyGraphWalker(cte->ctequery,cstate);
765-
/* note that recursion could mutate innerwiths list */
766-
cell1=list_head(cstate->innerwiths);
767-
lfirst(cell1)=lappend((List*)lfirst(cell1),cte);
768-
}
769-
(void)raw_expression_tree_walker(node,
770-
makeDependencyGraphWalker,
771-
(void*)cstate);
772-
cstate->innerwiths=list_delete_first(cstate->innerwiths);
773-
}
774-
/* We're done examining the SelectStmt */
756+
if (stmt->withClause)
757+
{
758+
/* Examine the WITH clause and the DeleteStmt */
759+
WalkInnerWith(node,stmt->withClause,cstate);
760+
/* We're done examining the DeleteStmt */
775761
return false;
776762
}
777763
/* if no WITH clause, just fall through for normal processing */
778764
}
779-
if (IsA(node,WithClause))
765+
elseif (IsA(node,UpdateStmt))
766+
{
767+
UpdateStmt*stmt= (UpdateStmt*)node;
768+
769+
if (stmt->withClause)
770+
{
771+
/* Examine the WITH clause and the UpdateStmt */
772+
WalkInnerWith(node,stmt->withClause,cstate);
773+
/* We're done examining the UpdateStmt */
774+
return false;
775+
}
776+
/* if no WITH clause, just fall through for normal processing */
777+
}
778+
elseif (IsA(node,MergeStmt))
779+
{
780+
MergeStmt*stmt= (MergeStmt*)node;
781+
782+
if (stmt->withClause)
783+
{
784+
/* Examine the WITH clause and the MergeStmt */
785+
WalkInnerWith(node,stmt->withClause,cstate);
786+
/* We're done examining the MergeStmt */
787+
return false;
788+
}
789+
/* if no WITH clause, just fall through for normal processing */
790+
}
791+
elseif (IsA(node,WithClause))
780792
{
781793
/*
782794
* Prevent raw_expression_tree_walker from recursing directly into a
@@ -790,6 +802,60 @@ makeDependencyGraphWalker(Node *node, CteState *cstate)
790802
(void*)cstate);
791803
}
792804

805+
/*
806+
* makeDependencyGraphWalker's recursion into a statement having a WITH clause.
807+
*
808+
* This subroutine is concerned with updating the innerwiths list correctly
809+
* based on the visibility rules for CTE names.
810+
*/
811+
staticvoid
812+
WalkInnerWith(Node*stmt,WithClause*withClause,CteState*cstate)
813+
{
814+
ListCell*lc;
815+
816+
if (withClause->recursive)
817+
{
818+
/*
819+
* In the RECURSIVE case, all query names of the WITH are visible to
820+
* all WITH items as well as the main query. So push them all on,
821+
* process, pop them all off.
822+
*/
823+
cstate->innerwiths=lcons(withClause->ctes,cstate->innerwiths);
824+
foreach(lc,withClause->ctes)
825+
{
826+
CommonTableExpr*cte= (CommonTableExpr*)lfirst(lc);
827+
828+
(void)makeDependencyGraphWalker(cte->ctequery,cstate);
829+
}
830+
(void)raw_expression_tree_walker(stmt,
831+
makeDependencyGraphWalker,
832+
(void*)cstate);
833+
cstate->innerwiths=list_delete_first(cstate->innerwiths);
834+
}
835+
else
836+
{
837+
/*
838+
* In the non-RECURSIVE case, query names are visible to the WITH
839+
* items after them and to the main query.
840+
*/
841+
cstate->innerwiths=lcons(NIL,cstate->innerwiths);
842+
foreach(lc,withClause->ctes)
843+
{
844+
CommonTableExpr*cte= (CommonTableExpr*)lfirst(lc);
845+
ListCell*cell1;
846+
847+
(void)makeDependencyGraphWalker(cte->ctequery,cstate);
848+
/* note that recursion could mutate innerwiths list */
849+
cell1=list_head(cstate->innerwiths);
850+
lfirst(cell1)=lappend((List*)lfirst(cell1),cte);
851+
}
852+
(void)raw_expression_tree_walker(stmt,
853+
makeDependencyGraphWalker,
854+
(void*)cstate);
855+
cstate->innerwiths=list_delete_first(cstate->innerwiths);
856+
}
857+
}
858+
793859
/*
794860
* Sort by dependencies, using a standard topological sort operation
795861
*/

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2084,6 +2084,14 @@ WITH RECURSIVE x(n) AS (
20842084
ERROR: ORDER BY in a recursive query is not implemented
20852085
LINE 3: ORDER BY (SELECT n FROM x))
20862086
^
2087+
-- and this
2088+
WITH RECURSIVE x(n) AS (
2089+
WITH sub_cte AS (SELECT * FROM x)
2090+
DELETE FROM graph RETURNING f)
2091+
SELECT * FROM x;
2092+
ERROR: recursive query "x" must not contain data-modifying statements
2093+
LINE 1: WITH RECURSIVE x(n) AS (
2094+
^
20872095
CREATE TEMPORARY TABLE y (a INTEGER);
20882096
INSERT INTO y SELECT generate_series(1, 10);
20892097
-- LEFT JOIN

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

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

952+
-- and this
953+
WITH RECURSIVE x(n)AS (
954+
WITH sub_cteAS (SELECT*FROM x)
955+
DELETEFROM graph RETURNING f)
956+
SELECT*FROM x;
957+
958+
952959
CREATE TEMPORARY TABLE y (aINTEGER);
953960
INSERT INTO ySELECT generate_series(1,10);
954961

0 commit comments

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