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

Commit9fa82c9

Browse files
committed
Fix planner's handling of RETURNING lists in writable CTEs.
setrefs.c failed to do "rtoffset" adjustment of Vars in RETURNING lists,which meant they were left with the wrong varnos when the RETURNING listwas in a subquery. That was never possible before writable CTEs, ofcourse, but now it's broken. The executor fails to notice any problembecause ExecEvalVar just references the ecxt_scantuple for any normalvarno; but EXPLAIN breaks when the varno is wrong, as illustrated in arecent complaint from Bartosz Dmytrak.Since the eventual rtoffset of the subquery is not known at the timewe are preparing its plan node, the previous scheme of executingset_returning_clause_references() at that time cannot handle thisadjustment. Fortunately, it turns out that we don't really need to do itthat way, because all the needed information is available during normalsetrefs.c execution; we just have to dig it out of the ModifyTable node.So, do that, and get rid of the kluge of early setrefs processing ofRETURNING lists. (This is a little bit of a cheat in the case of inheritedUPDATE/DELETE, because we are not passing a "root" struct that correspondsexactly to what the subplan was built with. But that doesn't matter, andanyway this is less ugly than early setrefs processing was.)Back-patch to 9.1, where the problem became possible to hit.
1 parentc62b8ea commit9fa82c9

File tree

6 files changed

+116
-50
lines changed

6 files changed

+116
-50
lines changed

‎src/backend/optimizer/plan/createplan.c

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4587,16 +4587,8 @@ make_modifytable(CmdType operation, bool canSetTag,
45874587
node->plan.lefttree=NULL;
45884588
node->plan.righttree=NULL;
45894589
node->plan.qual=NIL;
4590-
4591-
/*
4592-
* Set up the visible plan targetlist as being the same as the first
4593-
* RETURNING list.This is for the use of EXPLAIN; the executor won't pay
4594-
* any attention to the targetlist.
4595-
*/
4596-
if (returningLists)
4597-
node->plan.targetlist=copyObject(linitial(returningLists));
4598-
else
4599-
node->plan.targetlist=NIL;
4590+
/* setrefs.c will fill in the targetlist, if needed */
4591+
node->plan.targetlist=NIL;
46004592

46014593
node->operation=operation;
46024594
node->canSetTag=canSetTag;

‎src/backend/optimizer/plan/planner.c

Lines changed: 4 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -529,22 +529,10 @@ subquery_planner(PlannerGlobal *glob, Query *parse,
529529
List*rowMarks;
530530

531531
/*
532-
* Deal with the RETURNING clause if any. It's convenient to pass
533-
* the returningList through setrefs.c now rather than at top
534-
* level (if we waited, handling inherited UPDATE/DELETE would be
535-
* much harder).
532+
* Set up the RETURNING list-of-lists, if needed.
536533
*/
537534
if (parse->returningList)
538-
{
539-
List*rlist;
540-
541-
Assert(parse->resultRelation);
542-
rlist=set_returning_clause_references(root,
543-
parse->returningList,
544-
plan,
545-
parse->resultRelation);
546-
returningLists=list_make1(rlist);
547-
}
535+
returningLists=list_make1(parse->returningList);
548536
else
549537
returningLists=NIL;
550538

@@ -889,15 +877,8 @@ inheritance_planner(PlannerInfo *root)
889877

890878
/* Build list of per-relation RETURNING targetlists */
891879
if (parse->returningList)
892-
{
893-
List*rlist;
894-
895-
rlist=set_returning_clause_references(&subroot,
896-
subroot.parse->returningList,
897-
subplan,
898-
appinfo->child_relid);
899-
returningLists=lappend(returningLists,rlist);
900-
}
880+
returningLists=lappend(returningLists,
881+
subroot.parse->returningList);
901882
}
902883

903884
/* Mark result as unordered (probably unnecessary) */

‎src/backend/optimizer/plan/setrefs.c

Lines changed: 62 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,11 @@ static Node *fix_upper_expr(PlannerInfo *root,
121121
intrtoffset);
122122
staticNode*fix_upper_expr_mutator(Node*node,
123123
fix_upper_expr_context*context);
124+
staticList*set_returning_clause_references(PlannerInfo*root,
125+
List*rlist,
126+
Plan*topplan,
127+
IndexresultRelation,
128+
intrtoffset);
124129
staticboolfix_opfuncids_walker(Node*node,void*context);
125130
staticboolextract_query_dependencies_walker(Node*node,
126131
PlannerInfo*context);
@@ -548,13 +553,50 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset)
548553
{
549554
ModifyTable*splan= (ModifyTable*)plan;
550555

551-
/*
552-
* planner.c already called set_returning_clause_references,
553-
* so we should not process either the targetlist or the
554-
* returningLists.
555-
*/
556+
Assert(splan->plan.targetlist==NIL);
556557
Assert(splan->plan.qual==NIL);
557558

559+
if (splan->returningLists)
560+
{
561+
List*newRL=NIL;
562+
ListCell*lcrl,
563+
*lcrr,
564+
*lcp;
565+
566+
/*
567+
* Pass each per-subplan returningList through
568+
* set_returning_clause_references().
569+
*/
570+
Assert(list_length(splan->returningLists)==list_length(splan->resultRelations));
571+
Assert(list_length(splan->returningLists)==list_length(splan->plans));
572+
forthree(lcrl,splan->returningLists,
573+
lcrr,splan->resultRelations,
574+
lcp,splan->plans)
575+
{
576+
List*rlist= (List*)lfirst(lcrl);
577+
Indexresultrel=lfirst_int(lcrr);
578+
Plan*subplan= (Plan*)lfirst(lcp);
579+
580+
rlist=set_returning_clause_references(root,
581+
rlist,
582+
subplan,
583+
resultrel,
584+
rtoffset);
585+
newRL=lappend(newRL,rlist);
586+
}
587+
splan->returningLists=newRL;
588+
589+
/*
590+
* Set up the visible plan targetlist as being the same as
591+
* the first RETURNING list. This is for the use of
592+
* EXPLAIN; the executor won't pay any attention to the
593+
* targetlist. We postpone this step until here so that
594+
* we don't have to do set_returning_clause_references()
595+
* twice on identical targetlists.
596+
*/
597+
splan->plan.targetlist=copyObject(linitial(newRL));
598+
}
599+
558600
foreach(l,splan->resultRelations)
559601
{
560602
lfirst_int(l)+=rtoffset;
@@ -1532,6 +1574,7 @@ fix_join_expr_mutator(Node *node, fix_join_expr_context *context)
15321574
if (var->varno==context->acceptable_rel)
15331575
{
15341576
var=copyVar(var);
1577+
var->varno+=context->rtoffset;
15351578
if (var->varnoold>0)
15361579
var->varnoold+=context->rtoffset;
15371580
return (Node*)var;
@@ -1691,25 +1734,31 @@ fix_upper_expr_mutator(Node *node, fix_upper_expr_context *context)
16911734
* entries in the top subplan's targetlist. Vars referencing the result
16921735
* table should be left alone, however (the executor will evaluate them
16931736
* using the actual heap tuple, after firing triggers if any).In the
1694-
* adjusted RETURNING list, result-table Vars willstillhave their
1695-
*originalvarno, but Vars for other rels will have varno OUTER_VAR.
1737+
* adjusted RETURNING list, result-table Vars will have their original
1738+
* varno (plus rtoffset), but Vars for other rels will have varno OUTER_VAR.
16961739
*
16971740
* We also must perform opcode lookup and add regclass OIDs to
16981741
* root->glob->relationOids.
16991742
*
17001743
* 'rlist': the RETURNING targetlist to be fixed
17011744
* 'topplan': the top subplan node that will be just below the ModifyTable
1702-
*node (note it's not yet passed throughset_plan_references)
1745+
*node (note it's not yet passed throughset_plan_refs)
17031746
* 'resultRelation': RT index of the associated result relation
1747+
* 'rtoffset': how much to increment varnos by
17041748
*
1705-
* Note: we assume that result relations will have rtoffset zero, that is,
1706-
* they are not coming from a subplan.
1749+
* Note: the given 'root' is for the parent query level, not the 'topplan'.
1750+
* This does not matter currently since we only access the dependency-item
1751+
* lists in root->glob, but it would need some hacking if we wanted a root
1752+
* that actually matches the subplan.
1753+
*
1754+
* Note: resultRelation is not yet adjusted by rtoffset.
17071755
*/
1708-
List*
1756+
staticList*
17091757
set_returning_clause_references(PlannerInfo*root,
17101758
List*rlist,
17111759
Plan*topplan,
1712-
IndexresultRelation)
1760+
IndexresultRelation,
1761+
intrtoffset)
17131762
{
17141763
indexed_tlist*itlist;
17151764

@@ -1734,7 +1783,7 @@ set_returning_clause_references(PlannerInfo *root,
17341783
itlist,
17351784
NULL,
17361785
resultRelation,
1737-
0);
1786+
rtoffset);
17381787

17391788
pfree(itlist);
17401789

‎src/include/optimizer/planmain.h

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -120,10 +120,6 @@ extern List *remove_useless_joins(PlannerInfo *root, List *joinlist);
120120
* prototypes for plan/setrefs.c
121121
*/
122122
externPlan*set_plan_references(PlannerInfo*root,Plan*plan);
123-
externList*set_returning_clause_references(PlannerInfo*root,
124-
List*rlist,
125-
Plan*topplan,
126-
IndexresultRelation);
127123
externvoidfix_opfuncids(Node*node);
128124
externvoidset_opfuncid(OpExpr*opexpr);
129125
externvoidset_sa_opfuncid(ScalarArrayOpExpr*opexpr);

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

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1857,6 +1857,48 @@ SELECT * FROM parent;
18571857
42 | new2
18581858
(5 rows)
18591859

1860+
-- check EXPLAIN VERBOSE for a wCTE with RETURNING
1861+
EXPLAIN (VERBOSE, COSTS OFF)
1862+
WITH wcte AS ( INSERT INTO int8_tbl VALUES ( 42, 47 ) RETURNING q2 )
1863+
DELETE FROM a USING wcte WHERE aa = q2;
1864+
QUERY PLAN
1865+
--------------------------------------------------
1866+
Delete on public.a
1867+
CTE wcte
1868+
-> Insert on public.int8_tbl
1869+
Output: int8_tbl.q2
1870+
-> Result
1871+
Output: 42::bigint, 47::bigint
1872+
-> Nested Loop
1873+
Output: public.a.ctid, wcte.*
1874+
Join Filter: (public.a.aa = wcte.q2)
1875+
-> Seq Scan on public.a
1876+
Output: public.a.ctid, public.a.aa
1877+
-> CTE Scan on wcte
1878+
Output: wcte.*, wcte.q2
1879+
-> Nested Loop
1880+
Output: public.a.ctid, wcte.*
1881+
Join Filter: (public.a.aa = wcte.q2)
1882+
-> Seq Scan on public.b a
1883+
Output: public.a.ctid, public.a.aa
1884+
-> CTE Scan on wcte
1885+
Output: wcte.*, wcte.q2
1886+
-> Nested Loop
1887+
Output: public.a.ctid, wcte.*
1888+
Join Filter: (public.a.aa = wcte.q2)
1889+
-> Seq Scan on public.c a
1890+
Output: public.a.ctid, public.a.aa
1891+
-> CTE Scan on wcte
1892+
Output: wcte.*, wcte.q2
1893+
-> Nested Loop
1894+
Output: public.a.ctid, wcte.*
1895+
Join Filter: (public.a.aa = wcte.q2)
1896+
-> Seq Scan on public.d a
1897+
Output: public.a.ctid, public.a.aa
1898+
-> CTE Scan on wcte
1899+
Output: wcte.*, wcte.q2
1900+
(34 rows)
1901+
18601902
-- error cases
18611903
-- data-modifying WITH tries to use its own output
18621904
WITH RECURSIVE t AS (

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -791,6 +791,12 @@ DELETE FROM parent USING wcte WHERE id = newid;
791791

792792
SELECT*FROM parent;
793793

794+
-- check EXPLAIN VERBOSE for a wCTE with RETURNING
795+
796+
EXPLAIN (VERBOSE, COSTS OFF)
797+
WITH wcteAS (INSERT INTO int8_tblVALUES (42,47 ) RETURNING q2 )
798+
DELETEFROM a USING wcteWHERE aa= q2;
799+
794800
-- error cases
795801

796802
-- data-modifying WITH tries to use its own output

0 commit comments

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