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 635d42e9c),
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
This commit is contained in:
Tom Lane 2019-02-22 12:23:00 -05:00
parent 5a73edf050
commit 99a1554a26
5 changed files with 150 additions and 17 deletions

View File

@ -1087,29 +1087,47 @@ inheritance_planner(PlannerInfo *root)
/* Mark result as unordered (probably unnecessary) */ /* Mark result as unordered (probably unnecessary) */
root->query_pathkeys = NIL; root->query_pathkeys = NIL;
/*
* If we managed to exclude every child rel, return a dummy plan; it
* doesn't even need a ModifyTable node.
*/
if (subplans == NIL) if (subplans == NIL)
{ {
/* although dummy, it must have a valid tlist for executor */ /*
* We managed to exclude every child rel, so generate a dummy path
* representing the empty set. Although it's clear that no data will
* be updated or deleted, we will still need to have a ModifyTable
* node so that any statement triggers are executed. (This could be
* cleaner if we fixed nodeModifyTable.c to support zero child nodes,
* but that probably wouldn't be a net win.)
*/
List *tlist; List *tlist;
Plan *subplan;
/* tlist processing never got done, either */
tlist = preprocess_targetlist(root, parse->targetList); tlist = preprocess_targetlist(root, parse->targetList);
return (Plan *) make_result(root,
subplan = (Plan *) make_result(root,
tlist, tlist,
(Node *) list_make1(makeBoolConst(false, (Node *) list_make1(makeBoolConst(false,
false)), false)),
NULL); NULL);
}
/* These lists must be nonempty to make a valid ModifyTable node */
subplans = list_make1(subplan);
resultRelations = list_make1_int(parse->resultRelation);
if (parse->withCheckOptions)
withCheckOptionLists = list_make1(parse->withCheckOptions);
if (parse->returningList)
returningLists = list_make1(parse->returningList);
}
else
{
/* /*
* Put back the final adjusted rtable into the master copy of the Query. * Put back the final adjusted rtable into the master copy of the
* Query. (We mustn't do this if we found no non-excluded children,
* since we never saved an adjusted rtable at all.)
*/ */
parse->rtable = final_rtable; parse->rtable = final_rtable;
root->simple_rel_array_size = save_rel_array_size; root->simple_rel_array_size = save_rel_array_size;
root->simple_rel_array = save_rel_array; root->simple_rel_array = save_rel_array;
}
/* /*
* If there was a FOR [KEY] UPDATE/SHARE clause, the LockRows node will * If there was a FOR [KEY] UPDATE/SHARE clause, the LockRows node will

View File

@ -539,6 +539,45 @@ CREATE TEMP TABLE z (b TEXT, PRIMARY KEY(aa, b)) inherits (a);
INSERT INTO z VALUES (NULL, 'text'); -- should fail INSERT INTO z VALUES (NULL, 'text'); -- should fail
ERROR: null value in column "aa" violates not-null constraint ERROR: null value in column "aa" violates not-null constraint
DETAIL: Failing row contains (null, text). DETAIL: Failing row contains (null, text).
-- Check inherited UPDATE with all children excluded
create table some_tab (a int, b int);
create table some_tab_child () inherits (some_tab);
insert into some_tab_child values(1,2);
explain (verbose, costs off)
update some_tab set a = a + 1 where false;
QUERY PLAN
-------------------------------------------------------------
Update on public.some_tab
-> Result
Output: (some_tab.a + 1), some_tab.b, some_tab.ctid
One-Time Filter: false
(4 rows)
update some_tab set a = a + 1 where false;
explain (verbose, costs off)
update some_tab set a = a + 1 where false returning b, a;
QUERY PLAN
-------------------------------------------------------------
Update on public.some_tab
Output: some_tab.b, some_tab.a
-> Result
Output: (some_tab.a + 1), some_tab.b, some_tab.ctid
One-Time Filter: false
(5 rows)
update some_tab set a = a + 1 where false returning b, a;
b | a
---+---
(0 rows)
table some_tab;
a | b
---+---
1 | 2
(1 row)
drop table some_tab cascade;
NOTICE: drop cascades to table some_tab_child
-- Check UPDATE with inherited target and an inherited source table -- Check UPDATE with inherited target and an inherited source table
create temp table foo(f1 int, f2 int); create temp table foo(f1 int, f2 int);
create temp table foo2(f3 int) inherits (foo); create temp table foo2(f3 int) inherits (foo);

View File

@ -1757,3 +1757,37 @@ select * from self_ref_trigger;
drop table self_ref_trigger; drop table self_ref_trigger;
drop function self_ref_trigger_ins_func(); drop function self_ref_trigger_ins_func();
drop function self_ref_trigger_del_func(); drop function self_ref_trigger_del_func();
--
-- Check that statement triggers work correctly even with all children excluded
--
create table stmt_trig_on_empty_upd (a int);
create table stmt_trig_on_empty_upd1 () inherits (stmt_trig_on_empty_upd);
create function update_stmt_notice() returns trigger as $$
begin
raise notice 'updating %', TG_TABLE_NAME;
return null;
end;
$$ language plpgsql;
create trigger before_stmt_trigger
before update on stmt_trig_on_empty_upd
execute procedure update_stmt_notice();
create trigger before_stmt_trigger
before update on stmt_trig_on_empty_upd1
execute procedure update_stmt_notice();
-- inherited no-op update
update stmt_trig_on_empty_upd set a = a where false returning a+1 as aa;
NOTICE: updating stmt_trig_on_empty_upd
aa
----
(0 rows)
-- simple no-op update
update stmt_trig_on_empty_upd1 set a = a where false returning a+1 as aa;
NOTICE: updating stmt_trig_on_empty_upd1
aa
----
(0 rows)
drop table stmt_trig_on_empty_upd cascade;
NOTICE: drop cascades to table stmt_trig_on_empty_upd1
drop function update_stmt_notice();

View File

@ -97,6 +97,21 @@ SELECT relname, d.* FROM ONLY d, pg_class where d.tableoid = pg_class.oid;
CREATE TEMP TABLE z (b TEXT, PRIMARY KEY(aa, b)) inherits (a); CREATE TEMP TABLE z (b TEXT, PRIMARY KEY(aa, b)) inherits (a);
INSERT INTO z VALUES (NULL, 'text'); -- should fail INSERT INTO z VALUES (NULL, 'text'); -- should fail
-- Check inherited UPDATE with all children excluded
create table some_tab (a int, b int);
create table some_tab_child () inherits (some_tab);
insert into some_tab_child values(1,2);
explain (verbose, costs off)
update some_tab set a = a + 1 where false;
update some_tab set a = a + 1 where false;
explain (verbose, costs off)
update some_tab set a = a + 1 where false returning b, a;
update some_tab set a = a + 1 where false returning b, a;
table some_tab;
drop table some_tab cascade;
-- Check UPDATE with inherited target and an inherited source table -- Check UPDATE with inherited target and an inherited source table
create temp table foo(f1 int, f2 int); create temp table foo(f1 int, f2 int);
create temp table foo2(f3 int) inherits (foo); create temp table foo2(f3 int) inherits (foo);

View File

@ -1189,3 +1189,30 @@ select * from self_ref_trigger;
drop table self_ref_trigger; drop table self_ref_trigger;
drop function self_ref_trigger_ins_func(); drop function self_ref_trigger_ins_func();
drop function self_ref_trigger_del_func(); drop function self_ref_trigger_del_func();
--
-- Check that statement triggers work correctly even with all children excluded
--
create table stmt_trig_on_empty_upd (a int);
create table stmt_trig_on_empty_upd1 () inherits (stmt_trig_on_empty_upd);
create function update_stmt_notice() returns trigger as $$
begin
raise notice 'updating %', TG_TABLE_NAME;
return null;
end;
$$ language plpgsql;
create trigger before_stmt_trigger
before update on stmt_trig_on_empty_upd
execute procedure update_stmt_notice();
create trigger before_stmt_trigger
before update on stmt_trig_on_empty_upd1
execute procedure update_stmt_notice();
-- inherited no-op update
update stmt_trig_on_empty_upd set a = a where false returning a+1 as aa;
-- simple no-op update
update stmt_trig_on_empty_upd1 set a = a where false returning a+1 as aa;
drop table stmt_trig_on_empty_upd cascade;
drop function update_stmt_notice();