Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 07fba9a

Browse files
committed
Fix plan created for inherited UPDATE/DELETE with all tables excluded.
In the case where inheritance_planner() finds that every table has been excluded by constraints, it thought it could get away with making a plan consisting of just a dummy Result node. While certainly there's no updating or deleting to be done, this had two user-visible problems: the plan did not report the correct set of output columns when a RETURNING clause was present, and if there were any statement-level triggers that should be fired, it didn't fire them. Hence, rather than only generating the dummy Result, we need to stick a valid ModifyTable node on top, which requires a tad more effort here. It's been broken this way for as long as inheritance_planner() has known about deleting excluded subplans at all (cf commit 635d42e), so back-patch to all supported branches. Amit Langote and Tom Lane, per a report from Petr Fedorov. Discussion: https://postgr.es/m/5da6f0f0-1364-1876-6978-907678f89a3e@phystech.edu
1 parent 630de11 commit 07fba9a

File tree

5 files changed

+163
-22
lines changed

5 files changed

+163
-22
lines changed

src/backend/optimizer/plan/planner.c

Lines changed: 46 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1570,34 +1570,58 @@ inheritance_planner(PlannerInfo *root)
15701570
* to get control here.
15711571
*/
15721572

1573-
/*
1574-
* If we managed to exclude every child rel, return a dummy plan; it
1575-
* doesn't even need a ModifyTable node.
1576-
*/
15771573
if (subpaths == NIL)
15781574
{
1579-
set_dummy_rel_pathlist(final_rel);
1580-
return;
1581-
}
1575+
/*
1576+
* We managed to exclude every child rel, so generate a dummy path
1577+
* representing the empty set. Although it's clear that no data will
1578+
* be updated or deleted, we will still need to have a ModifyTable
1579+
* node so that any statement triggers are executed. (This could be
1580+
* cleaner if we fixed nodeModifyTable.c to support zero child nodes,
1581+
* but that probably wouldn't be a net win.)
1582+
*/
1583+
List *tlist;
1584+
Path *dummy_path;
15821585

1583-
/*
1584-
* Put back the final adjusted rtable into the master copy of the Query.
1585-
* (We mustn't do this if we found no non-excluded children.)
1586-
*/
1587-
parse->rtable = final_rtable;
1588-
root->simple_rel_array_size = save_rel_array_size;
1589-
root->simple_rel_array = save_rel_array;
1590-
root->append_rel_array = save_append_rel_array;
1586+
/* tlist processing never got done, either */
1587+
tlist = root->processed_tlist = preprocess_targetlist(root);
1588+
final_rel->reltarget = create_pathtarget(root, tlist);
15911589

1592-
/* Must reconstruct master's simple_rte_array, too */
1593-
root->simple_rte_array = (RangeTblEntry **)
1594-
palloc0((list_length(final_rtable) + 1) * sizeof(RangeTblEntry *));
1595-
rti = 1;
1596-
foreach(lc, final_rtable)
1590+
/* Make a dummy path, cf set_dummy_rel_pathlist() */
1591+
dummy_path = (Path *) create_append_path(NULL, final_rel, NIL, NIL,
1592+
NULL, 0, false, NIL, -1);
1593+
1594+
/* These lists must be nonempty to make a valid ModifyTable node */
1595+
subpaths = list_make1(dummy_path);
1596+
subroots = list_make1(root);
1597+
resultRelations = list_make1_int(parse->resultRelation);
1598+
if (parse->withCheckOptions)
1599+
withCheckOptionLists = list_make1(parse->withCheckOptions);
1600+
if (parse->returningList)
1601+
returningLists = list_make1(parse->returningList);
1602+
}
1603+
else
15971604
{
1598-
RangeTblEntry *rte = lfirst_node(RangeTblEntry, lc);
1605+
/*
1606+
* Put back the final adjusted rtable into the master copy of the
1607+
* Query. (We mustn't do this if we found no non-excluded children,
1608+
* since we never saved an adjusted rtable at all.)
1609+
*/
1610+
parse->rtable = final_rtable;
1611+
root->simple_rel_array_size = save_rel_array_size;
1612+
root->simple_rel_array = save_rel_array;
1613+
root->append_rel_array = save_append_rel_array;
1614+
1615+
/* Must reconstruct master's simple_rte_array, too */
1616+
root->simple_rte_array = (RangeTblEntry **)
1617+
palloc0((list_length(final_rtable) + 1) * sizeof(RangeTblEntry *));
1618+
rti = 1;
1619+
foreach(lc, final_rtable)
1620+
{
1621+
RangeTblEntry *rte = lfirst_node(RangeTblEntry, lc);
15991622

1600-
root->simple_rte_array[rti++] = rte;
1623+
root->simple_rte_array[rti++] = rte;
1624+
}
16011625
}
16021626

16031627
/*

src/test/regress/expected/inherit.out

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -539,6 +539,47 @@ CREATE TEMP TABLE z (b TEXT, PRIMARY KEY(aa, b)) inherits (a);
539539
INSERT INTO z VALUES (NULL, 'text'); -- should fail
540540
ERROR: null value in column "aa" violates not-null constraint
541541
DETAIL: Failing row contains (null, text).
542+
-- Check inherited UPDATE with all children excluded
543+
create table some_tab (a int, b int);
544+
create table some_tab_child () inherits (some_tab);
545+
insert into some_tab_child values(1,2);
546+
explain (verbose, costs off)
547+
update some_tab set a = a + 1 where false;
548+
QUERY PLAN
549+
----------------------------------
550+
Update on public.some_tab
551+
Update on public.some_tab
552+
-> Result
553+
Output: (a + 1), b, ctid
554+
One-Time Filter: false
555+
(5 rows)
556+
557+
update some_tab set a = a + 1 where false;
558+
explain (verbose, costs off)
559+
update some_tab set a = a + 1 where false returning b, a;
560+
QUERY PLAN
561+
----------------------------------
562+
Update on public.some_tab
563+
Output: b, a
564+
Update on public.some_tab
565+
-> Result
566+
Output: (a + 1), b, ctid
567+
One-Time Filter: false
568+
(6 rows)
569+
570+
update some_tab set a = a + 1 where false returning b, a;
571+
b | a
572+
---+---
573+
(0 rows)
574+
575+
table some_tab;
576+
a | b
577+
---+---
578+
1 | 2
579+
(1 row)
580+
581+
drop table some_tab cascade;
582+
NOTICE: drop cascades to table some_tab_child
542583
-- Check UPDATE with inherited target and an inherited source table
543584
create temp table foo(f1 int, f2 int);
544585
create temp table foo2(f3 int) inherits (foo);

src/test/regress/expected/triggers.out

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1714,6 +1714,40 @@ drop table self_ref_trigger;
17141714
drop function self_ref_trigger_ins_func();
17151715
drop function self_ref_trigger_del_func();
17161716
--
1717+
-- Check that statement triggers work correctly even with all children excluded
1718+
--
1719+
create table stmt_trig_on_empty_upd (a int);
1720+
create table stmt_trig_on_empty_upd1 () inherits (stmt_trig_on_empty_upd);
1721+
create function update_stmt_notice() returns trigger as $$
1722+
begin
1723+
raise notice 'updating %', TG_TABLE_NAME;
1724+
return null;
1725+
end;
1726+
$$ language plpgsql;
1727+
create trigger before_stmt_trigger
1728+
before update on stmt_trig_on_empty_upd
1729+
execute procedure update_stmt_notice();
1730+
create trigger before_stmt_trigger
1731+
before update on stmt_trig_on_empty_upd1
1732+
execute procedure update_stmt_notice();
1733+
-- inherited no-op update
1734+
update stmt_trig_on_empty_upd set a = a where false returning a+1 as aa;
1735+
NOTICE: updating stmt_trig_on_empty_upd
1736+
aa
1737+
----
1738+
(0 rows)
1739+
1740+
-- simple no-op update
1741+
update stmt_trig_on_empty_upd1 set a = a where false returning a+1 as aa;
1742+
NOTICE: updating stmt_trig_on_empty_upd1
1743+
aa
1744+
----
1745+
(0 rows)
1746+
1747+
drop table stmt_trig_on_empty_upd cascade;
1748+
NOTICE: drop cascades to table stmt_trig_on_empty_upd1
1749+
drop function update_stmt_notice();
1750+
--
17171751
-- Check that index creation (or DDL in general) is prohibited in a trigger
17181752
--
17191753
create table trigger_ddl_table (

src/test/regress/sql/inherit.sql

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,21 @@ SELECT relname, d.* FROM ONLY d, pg_class where d.tableoid = pg_class.oid;
9797
CREATE TEMP TABLE z (b TEXT, PRIMARY KEY(aa, b)) inherits (a);
9898
INSERT INTO z VALUES (NULL, 'text'); -- should fail
9999

100+
-- Check inherited UPDATE with all children excluded
101+
create table some_tab (a int, b int);
102+
create table some_tab_child () inherits (some_tab);
103+
insert into some_tab_child values(1,2);
104+
105+
explain (verbose, costs off)
106+
update some_tab set a = a + 1 where false;
107+
update some_tab set a = a + 1 where false;
108+
explain (verbose, costs off)
109+
update some_tab set a = a + 1 where false returning b, a;
110+
update some_tab set a = a + 1 where false returning b, a;
111+
table some_tab;
112+
113+
drop table some_tab cascade;
114+
100115
-- Check UPDATE with inherited target and an inherited source table
101116
create temp table foo(f1 int, f2 int);
102117
create temp table foo2(f3 int) inherits (foo);

src/test/regress/sql/triggers.sql

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1182,6 +1182,33 @@ drop table self_ref_trigger;
11821182
drop function self_ref_trigger_ins_func();
11831183
drop function self_ref_trigger_del_func();
11841184

1185+
--
1186+
-- Check that statement triggers work correctly even with all children excluded
1187+
--
1188+
1189+
create table stmt_trig_on_empty_upd (a int);
1190+
create table stmt_trig_on_empty_upd1 () inherits (stmt_trig_on_empty_upd);
1191+
create function update_stmt_notice() returns trigger as $$
1192+
begin
1193+
raise notice 'updating %', TG_TABLE_NAME;
1194+
return null;
1195+
end;
1196+
$$ language plpgsql;
1197+
create trigger before_stmt_trigger
1198+
before update on stmt_trig_on_empty_upd
1199+
execute procedure update_stmt_notice();
1200+
create trigger before_stmt_trigger
1201+
before update on stmt_trig_on_empty_upd1
1202+
execute procedure update_stmt_notice();
1203+
1204+
-- inherited no-op update
1205+
update stmt_trig_on_empty_upd set a = a where false returning a+1 as aa;
1206+
-- simple no-op update
1207+
update stmt_trig_on_empty_upd1 set a = a where false returning a+1 as aa;
1208+
1209+
drop table stmt_trig_on_empty_upd cascade;
1210+
drop function update_stmt_notice();
1211+
11851212
--
11861213
-- Check that index creation (or DDL in general) is prohibited in a trigger
11871214
--

0 commit comments

Comments
 (0)