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

Commit 387f9ed

Browse files
committed
Fix problems when a plain-inheritance parent table is excluded.
When an UPDATE/DELETE/MERGE's target table is an old-style inheritance tree, it's possible for the parent to get excluded from the plan while some children are not. (I believe this is only possible if we can prove that a CHECK ... NO INHERIT constraint on the parent contradicts the query WHERE clause, so it's a very unusual case.) In such a case, ExecInitModifyTable mistakenly concluded that the first surviving child is the target table, leading to at least two bugs: 1. The wrong table's statement-level triggers would get fired. 2. In v16 and up, it was possible to fail with "invalid perminfoindex 0 in RTE with relid nnnn" due to the child RTE not having permissions data included in the query plan. This was hard to reproduce reliably because it did not occur unless the update triggered some non-HOT index updates. In v14 and up, this is easy to fix by defining ModifyTable.rootRelation to be the parent RTE in plain inheritance as well as partitioned cases. While the wrong-triggers bug also appears in older branches, the relevant code in both the planner and executor is quite a bit different, so it would take a good deal of effort to develop and test a suitable patch. Given the lack of field complaints about the trigger issue, I'll desist for now. (Patching v11 for this seems unwise anyway, given that it will have no more releases after next month.) Per bug #18147 from Hans Buschmann. Amit Langote and Tom Lane Discussion: https://postgr.es/m/18147-6fc796538913ee88@postgresql.org
1 parent 74e5ea1 commit 387f9ed

File tree

7 files changed

+64
-22
lines changed

7 files changed

+64
-22
lines changed

src/backend/executor/nodeModifyTable.c

+5-4
Original file line numberDiff line numberDiff line change
@@ -3966,10 +3966,10 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
39663966
* must be converted, and
39673967
* - the root partitioned table used for tuple routing.
39683968
*
3969-
* If it's a partitioned table, the root partition doesn't appear
3970-
* elsewhere in the plan and its RT index is given explicitly in
3971-
* node->rootRelation. Otherwise (i.e. table inheritance) the target
3972-
* relation is the first relation in the node->resultRelations list.
3969+
* If it's a partitioned or inherited table, the root partition or
3970+
* appendrel RTE doesn't appear elsewhere in the plan and its RT index is
3971+
* given explicitly in node->rootRelation. Otherwise, the target relation
3972+
* is the sole relation in the node->resultRelations list.
39733973
*----------
39743974
*/
39753975
if (node->rootRelation > 0)
@@ -3980,6 +3980,7 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
39803980
}
39813981
else
39823982
{
3983+
Assert(list_length(node->resultRelations) == 1);
39833984
mtstate->rootResultRelInfo = mtstate->resultRelInfo;
39843985
ExecInitResultRelation(estate, mtstate->resultRelInfo,
39853986
linitial_int(node->resultRelations));

src/backend/optimizer/plan/planner.c

+4-10
Original file line numberDiff line numberDiff line change
@@ -1800,6 +1800,9 @@ grouping_planner(PlannerInfo *root, double tuple_fraction)
18001800
parse->resultRelation);
18011801
int resultRelation = -1;
18021802

1803+
/* Pass the root result rel forward to the executor. */
1804+
rootRelation = parse->resultRelation;
1805+
18031806
/* Add only leaf children to ModifyTable. */
18041807
while ((resultRelation = bms_next_member(root->leaf_result_relids,
18051808
resultRelation)) >= 0)
@@ -1923,6 +1926,7 @@ grouping_planner(PlannerInfo *root, double tuple_fraction)
19231926
else
19241927
{
19251928
/* Single-relation INSERT/UPDATE/DELETE/MERGE. */
1929+
rootRelation = 0; /* there's no separate root rel */
19261930
resultRelations = list_make1_int(parse->resultRelation);
19271931
if (parse->commandType == CMD_UPDATE)
19281932
updateColnosLists = list_make1(root->update_colnos);
@@ -1934,16 +1938,6 @@ grouping_planner(PlannerInfo *root, double tuple_fraction)
19341938
mergeActionLists = list_make1(parse->mergeActionList);
19351939
}
19361940

1937-
/*
1938-
* If target is a partition root table, we need to mark the
1939-
* ModifyTable node appropriately for that.
1940-
*/
1941-
if (rt_fetch(parse->resultRelation, parse->rtable)->relkind ==
1942-
RELKIND_PARTITIONED_TABLE)
1943-
rootRelation = parse->resultRelation;
1944-
else
1945-
rootRelation = 0;
1946-
19471941
/*
19481942
* If there was a FOR [KEY] UPDATE/SHARE clause, the LockRows node
19491943
* will have dealt with fetching non-locked marked rows, else we

src/backend/optimizer/util/pathnode.c

+1-1
Original file line numberDiff line numberDiff line change
@@ -3663,7 +3663,7 @@ create_lockrows_path(PlannerInfo *root, RelOptInfo *rel,
36633663
* 'operation' is the operation type
36643664
* 'canSetTag' is true if we set the command tag/es_processed
36653665
* 'nominalRelation' is the parent RT index for use of EXPLAIN
3666-
* 'rootRelation' is the partitioned table root RT index, or 0 if none
3666+
* 'rootRelation' is the partitioned/inherited table root RTI, or 0 if none
36673667
* 'partColsUpdated' is true if any partitioning columns are being updated,
36683668
* either from the target relation or a descendent partitioned table.
36693669
* 'resultRelations' is an integer list of actual RT indexes of target rel(s)

src/include/nodes/pathnodes.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -2344,7 +2344,7 @@ typedef struct ModifyTablePath
23442344
CmdType operation; /* INSERT, UPDATE, DELETE, or MERGE */
23452345
bool canSetTag; /* do we set the command tag/es_processed? */
23462346
Index nominalRelation; /* Parent RT index for use of EXPLAIN */
2347-
Index rootRelation; /* Root RT index, if target is partitioned */
2347+
Index rootRelation; /* Root RT index, if partitioned/inherited */
23482348
bool partColsUpdated; /* some part key in hierarchy updated? */
23492349
List *resultRelations; /* integer list of RT indexes */
23502350
List *updateColnosLists; /* per-target-table update_colnos lists */

src/include/nodes/plannodes.h

+7-6
Original file line numberDiff line numberDiff line change
@@ -216,11 +216,12 @@ typedef struct ProjectSet
216216
* Apply rows produced by outer plan to result table(s),
217217
* by inserting, updating, or deleting.
218218
*
219-
* If the originally named target table is a partitioned table, both
220-
* nominalRelation and rootRelation contain the RT index of the partition
221-
* root, which is not otherwise mentioned in the plan. Otherwise rootRelation
222-
* is zero. However, nominalRelation will always be set, as it's the rel that
223-
* EXPLAIN should claim is the INSERT/UPDATE/DELETE/MERGE target.
219+
* If the originally named target table is a partitioned table or inheritance
220+
* tree, both nominalRelation and rootRelation contain the RT index of the
221+
* partition root or appendrel RTE, which is not otherwise mentioned in the
222+
* plan. Otherwise rootRelation is zero. However, nominalRelation will
223+
* always be set, as it's the rel that EXPLAIN should claim is the
224+
* INSERT/UPDATE/DELETE/MERGE target.
224225
*
225226
* Note that rowMarks and epqParam are presumed to be valid for all the
226227
* table(s); they can't contain any info that varies across tables.
@@ -232,7 +233,7 @@ typedef struct ModifyTable
232233
CmdType operation; /* INSERT, UPDATE, DELETE, or MERGE */
233234
bool canSetTag; /* do we set the command tag/es_processed? */
234235
Index nominalRelation; /* Parent RT index for use of EXPLAIN */
235-
Index rootRelation; /* Root RT index, if target is partitioned */
236+
Index rootRelation; /* Root RT index, if partitioned/inherited */
236237
bool partColsUpdated; /* some part key in hierarchy updated? */
237238
List *resultRelations; /* integer list of RT indexes */
238239
List *updateColnosLists; /* per-target-table update_colnos lists */

src/test/regress/expected/inherit.out

+27
Original file line numberDiff line numberDiff line change
@@ -539,6 +539,33 @@ 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" of relation "z" violates not-null constraint
541541
DETAIL: Failing row contains (null, text).
542+
-- Check inherited UPDATE with first child excluded
543+
create table some_tab (f1 int, f2 int, f3 int, check (f1 < 10) no inherit);
544+
create table some_tab_child () inherits(some_tab);
545+
insert into some_tab_child select i, i+1, 0 from generate_series(1,1000) i;
546+
create index on some_tab_child(f1, f2);
547+
-- while at it, also check that statement-level triggers fire
548+
create function some_tab_stmt_trig_func() returns trigger as
549+
$$begin raise notice 'updating some_tab'; return NULL; end;$$
550+
language plpgsql;
551+
create trigger some_tab_stmt_trig
552+
before update on some_tab execute function some_tab_stmt_trig_func();
553+
explain (costs off)
554+
update some_tab set f3 = 11 where f1 = 12 and f2 = 13;
555+
QUERY PLAN
556+
------------------------------------------------------------------------------------
557+
Update on some_tab
558+
Update on some_tab_child some_tab_1
559+
-> Result
560+
-> Index Scan using some_tab_child_f1_f2_idx on some_tab_child some_tab_1
561+
Index Cond: ((f1 = 12) AND (f2 = 13))
562+
(5 rows)
563+
564+
update some_tab set f3 = 11 where f1 = 12 and f2 = 13;
565+
NOTICE: updating some_tab
566+
drop table some_tab cascade;
567+
NOTICE: drop cascades to table some_tab_child
568+
drop function some_tab_stmt_trig_func();
542569
-- Check inherited UPDATE with all children excluded
543570
create table some_tab (a int, b int);
544571
create table some_tab_child () inherits (some_tab);

src/test/regress/sql/inherit.sql

+19
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,25 @@ 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 first child excluded
101+
create table some_tab (f1 int, f2 int, f3 int, check (f1 < 10) no inherit);
102+
create table some_tab_child () inherits(some_tab);
103+
insert into some_tab_child select i, i+1, 0 from generate_series(1,1000) i;
104+
create index on some_tab_child(f1, f2);
105+
-- while at it, also check that statement-level triggers fire
106+
create function some_tab_stmt_trig_func() returns trigger as
107+
$$begin raise notice 'updating some_tab'; return NULL; end;$$
108+
language plpgsql;
109+
create trigger some_tab_stmt_trig
110+
before update on some_tab execute function some_tab_stmt_trig_func();
111+
112+
explain (costs off)
113+
update some_tab set f3 = 11 where f1 = 12 and f2 = 13;
114+
update some_tab set f3 = 11 where f1 = 12 and f2 = 13;
115+
116+
drop table some_tab cascade;
117+
drop function some_tab_stmt_trig_func();
118+
100119
-- Check inherited UPDATE with all children excluded
101120
create table some_tab (a int, b int);
102121
create table some_tab_child () inherits (some_tab);

0 commit comments

Comments
 (0)