Skip to content

Commit ab5fcf2

Browse files
tglsfdctglsfdc
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 98098fa commit ab5fcf2

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
@@ -1583,34 +1583,58 @@ inheritance_planner(PlannerInfo *root)
15831583
* to get control here.
15841584
*/
15851585

1586-
/*
1587-
* If we managed to exclude every child rel, return a dummy plan; it
1588-
* doesn't even need a ModifyTable node.
1589-
*/
15901586
if (subpaths == NIL)
15911587
{
1592-
set_dummy_rel_pathlist(final_rel);
1593-
return;
1594-
}
1588+
/*
1589+
* We managed to exclude every child rel, so generate a dummy path
1590+
* representing the empty set. Although it's clear that no data will
1591+
* be updated or deleted, we will still need to have a ModifyTable
1592+
* node so that any statement triggers are executed. (This could be
1593+
* cleaner if we fixed nodeModifyTable.c to support zero child nodes,
1594+
* but that probably wouldn't be a net win.)
1595+
*/
1596+
List *tlist;
1597+
Path *dummy_path;
15951598

1596-
/*
1597-
* Put back the final adjusted rtable into the master copy of the Query.
1598-
* (We mustn't do this if we found no non-excluded children.)
1599-
*/
1600-
parse->rtable = final_rtable;
1601-
root->simple_rel_array_size = save_rel_array_size;
1602-
root->simple_rel_array = save_rel_array;
1603-
root->append_rel_array = save_append_rel_array;
1599+
/* tlist processing never got done, either */
1600+
tlist = root->processed_tlist = preprocess_targetlist(root);
1601+
final_rel->reltarget = create_pathtarget(root, tlist);
16041602

1605-
/* Must reconstruct master's simple_rte_array, too */
1606-
root->simple_rte_array = (RangeTblEntry **)
1607-
palloc0((list_length(final_rtable) + 1) * sizeof(RangeTblEntry *));
1608-
rti = 1;
1609-
foreach(lc, final_rtable)
1603+
/* Make a dummy path, cf set_dummy_rel_pathlist() */
1604+
dummy_path = (Path *) create_append_path(NULL, final_rel, NIL, NIL,
1605+
NULL, 0, false, NIL, -1);
1606+
1607+
/* These lists must be nonempty to make a valid ModifyTable node */
1608+
subpaths = list_make1(dummy_path);
1609+
subroots = list_make1(root);
1610+
resultRelations = list_make1_int(parse->resultRelation);
1611+
if (parse->withCheckOptions)
1612+
withCheckOptionLists = list_make1(parse->withCheckOptions);
1613+
if (parse->returningList)
1614+
returningLists = list_make1(parse->returningList);
1615+
}
1616+
else
16101617
{
1611-
RangeTblEntry *rte = lfirst_node(RangeTblEntry, lc);
1618+
/*
1619+
* Put back the final adjusted rtable into the master copy of the
1620+
* Query. (We mustn't do this if we found no non-excluded children,
1621+
* since we never saved an adjusted rtable at all.)
1622+
*/
1623+
parse->rtable = final_rtable;
1624+
root->simple_rel_array_size = save_rel_array_size;
1625+
root->simple_rel_array = save_rel_array;
1626+
root->append_rel_array = save_append_rel_array;
1627+
1628+
/* Must reconstruct master's simple_rte_array, too */
1629+
root->simple_rte_array = (RangeTblEntry **)
1630+
palloc0((list_length(final_rtable) + 1) * sizeof(RangeTblEntry *));
1631+
rti = 1;
1632+
foreach(lc, final_rtable)
1633+
{
1634+
RangeTblEntry *rte = lfirst_node(RangeTblEntry, lc);
16121635

1613-
root->simple_rte_array[rti++] = rte;
1636+
root->simple_rte_array[rti++] = rte;
1637+
}
16141638
}
16151639

16161640
/*

‎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
@@ -1716,6 +1716,40 @@ drop table self_ref_trigger;
17161716
drop function self_ref_trigger_ins_func();
17171717
drop function self_ref_trigger_del_func();
17181718
--
1719+
-- Check that statement triggers work correctly even with all children excluded
1720+
--
1721+
create table stmt_trig_on_empty_upd (a int);
1722+
create table stmt_trig_on_empty_upd1 () inherits (stmt_trig_on_empty_upd);
1723+
create function update_stmt_notice() returns trigger as $$
1724+
begin
1725+
raise notice 'updating %', TG_TABLE_NAME;
1726+
return null;
1727+
end;
1728+
$$ language plpgsql;
1729+
create trigger before_stmt_trigger
1730+
before update on stmt_trig_on_empty_upd
1731+
execute procedure update_stmt_notice();
1732+
create trigger before_stmt_trigger
1733+
before update on stmt_trig_on_empty_upd1
1734+
execute procedure update_stmt_notice();
1735+
-- inherited no-op update
1736+
update stmt_trig_on_empty_upd set a = a where false returning a+1 as aa;
1737+
NOTICE: updating stmt_trig_on_empty_upd
1738+
aa
1739+
----
1740+
(0 rows)
1741+
1742+
-- simple no-op update
1743+
update stmt_trig_on_empty_upd1 set a = a where false returning a+1 as aa;
1744+
NOTICE: updating stmt_trig_on_empty_upd1
1745+
aa
1746+
----
1747+
(0 rows)
1748+
1749+
drop table stmt_trig_on_empty_upd cascade;
1750+
NOTICE: drop cascades to table stmt_trig_on_empty_upd1
1751+
drop function update_stmt_notice();
1752+
--
17191753
-- Check that index creation (or DDL in general) is prohibited in a trigger
17201754
--
17211755
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
@@ -1177,6 +1177,33 @@ drop table self_ref_trigger;
11771177
drop function self_ref_trigger_ins_func();
11781178
drop function self_ref_trigger_del_func();
11791179

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

0 commit comments

Comments
 (0)