Skip to content

Commit 2992b9a

Browse files
david-rowleydavid-rowley
committed
Fix incorrect logic for caching ResultRelInfos for triggers
When dealing with ResultRelInfos for partitions, there are cases where there are mixed requirements for the ri_RootResultRelInfo. There are cases when the partition itself requires a NULL ri_RootResultRelInfo and in the same query, the same partition may require a ResultRelInfo with its parent set in ri_RootResultRelInfo. This could cause the column mapping between the partitioned table and the partition not to be done which could result in crashes if the column attnums didn't match exactly. The fix is simple. We now check that the ri_RootResultRelInfo matches what the caller passed to ExecGetTriggerResultRel() and only return a cached ResultRelInfo when the ri_RootResultRelInfo matches what the caller wants, otherwise we'll make a new one. Author: David Rowley <dgrowleyml@gmail.com> Author: Amit Langote <amitlangote09@gmail.com> Reported-by: Dmitry Fomin <fomin.list@gmail.com> Discussion: https://postgr.es/m/7DCE78D7-0520-4207-822B-92F60AEA14B4@gmail.com Backpatch-through: 15
1 parent 5f0b6f7 commit 2992b9a

File tree

3 files changed

+116
-7
lines changed

3 files changed

+116
-7
lines changed

‎src/backend/executor/execMain.c‎

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1310,10 +1310,9 @@ InitResultRelInfo(ResultRelInfo *resultRelInfo,
13101310
* Get a ResultRelInfo for a trigger target relation.
13111311
*
13121312
* Most of the time, triggers are fired on one of the result relations of the
1313-
* query, and so we can just return a member of the es_result_relations array,
1314-
* or the es_tuple_routing_result_relations list (if any). (Note: in self-join
1315-
* situations there might be multiple members with the same OID; if so it
1316-
* doesn't matter which one we pick.)
1313+
* query, and so we can just return a suitable one we already made and stored
1314+
* in the es_opened_result_relations or es_tuple_routing_result_relations
1315+
* Lists.
13171316
*
13181317
* However, it is sometimes necessary to fire triggers on other relations;
13191318
* this happens mainly when an RI update trigger queues additional triggers
@@ -1333,11 +1332,20 @@ ExecGetTriggerResultRel(EState *estate, Oid relid,
13331332
Relation rel;
13341333
MemoryContext oldcontext;
13351334

1335+
/*
1336+
* Before creating a new ResultRelInfo, check if we've already made and
1337+
* cached one for this relation. We must ensure that the given
1338+
* 'rootRelInfo' matches the one stored in the cached ResultRelInfo as
1339+
* trigger handling for partitions can result in mixed requirements for
1340+
* what ri_RootResultRelInfo is set to.
1341+
*/
1342+
13361343
/* Search through the query result relations */
13371344
foreach(l, estate->es_opened_result_relations)
13381345
{
13391346
rInfo = lfirst(l);
1340-
if (RelationGetRelid(rInfo->ri_RelationDesc) == relid)
1347+
if (RelationGetRelid(rInfo->ri_RelationDesc) == relid &&
1348+
rInfo->ri_RootResultRelInfo == rootRelInfo)
13411349
return rInfo;
13421350
}
13431351

@@ -1348,15 +1356,17 @@ ExecGetTriggerResultRel(EState *estate, Oid relid,
13481356
foreach(l, estate->es_tuple_routing_result_relations)
13491357
{
13501358
rInfo = (ResultRelInfo *) lfirst(l);
1351-
if (RelationGetRelid(rInfo->ri_RelationDesc) == relid)
1359+
if (RelationGetRelid(rInfo->ri_RelationDesc) == relid &&
1360+
rInfo->ri_RootResultRelInfo == rootRelInfo)
13521361
return rInfo;
13531362
}
13541363

13551364
/* Nope, but maybe we already made an extra ResultRelInfo for it */
13561365
foreach(l, estate->es_trig_target_relations)
13571366
{
13581367
rInfo = (ResultRelInfo *) lfirst(l);
1359-
if (RelationGetRelid(rInfo->ri_RelationDesc) == relid)
1368+
if (RelationGetRelid(rInfo->ri_RelationDesc) == relid &&
1369+
rInfo->ri_RootResultRelInfo == rootRelInfo)
13601370
return rInfo;
13611371
}
13621372
/* Nope, so we need a new one */

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

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3077,3 +3077,54 @@ SET client_min_messages TO warning;
30773077
DROP SCHEMA fkpart12 CASCADE;
30783078
RESET client_min_messages;
30793079
RESET search_path;
3080+
-- Exercise the column mapping code with foreign keys. In this test we'll
3081+
-- create a partitioned table which has a partition with a dropped column and
3082+
-- check to ensure that an UPDATE cascades the changes correctly to the
3083+
-- partitioned table.
3084+
CREATE SCHEMA fkpart13;
3085+
SET search_path TO fkpart13;
3086+
CREATE TABLE fkpart13_t1 (a int PRIMARY KEY);
3087+
CREATE TABLE fkpart13_t2 (
3088+
part_id int PRIMARY KEY,
3089+
column_to_drop int,
3090+
FOREIGN KEY (part_id) REFERENCES fkpart13_t1 ON UPDATE CASCADE ON DELETE CASCADE
3091+
) PARTITION BY LIST (part_id);
3092+
CREATE TABLE fkpart13_t2_p1 PARTITION OF fkpart13_t2 FOR VALUES IN (1);
3093+
-- drop the column
3094+
ALTER TABLE fkpart13_t2 DROP COLUMN column_to_drop;
3095+
-- create a new partition without the dropped column
3096+
CREATE TABLE fkpart13_t2_p2 PARTITION OF fkpart13_t2 FOR VALUES IN (2);
3097+
CREATE TABLE fkpart13_t3 (
3098+
a int NOT NULL,
3099+
FOREIGN KEY (a)
3100+
REFERENCES fkpart13_t2
3101+
ON UPDATE CASCADE ON DELETE CASCADE
3102+
);
3103+
INSERT INTO fkpart13_t1 (a) VALUES (1);
3104+
INSERT INTO fkpart13_t2 (part_id) VALUES (1);
3105+
INSERT INTO fkpart13_t3 (a) VALUES (1);
3106+
-- Test a cascading update works correctly with with the dropped column
3107+
UPDATE fkpart13_t1 SET a = 2 WHERE a = 1;
3108+
SELECT tableoid::regclass,* FROM fkpart13_t2;
3109+
tableoid | part_id
3110+
----------------+---------
3111+
fkpart13_t2_p2 | 2
3112+
(1 row)
3113+
3114+
SELECT tableoid::regclass,* FROM fkpart13_t3;
3115+
tableoid | a
3116+
-------------+---
3117+
fkpart13_t3 | 2
3118+
(1 row)
3119+
3120+
-- Exercise code in ExecGetTriggerResultRel() as there's been previous issues
3121+
-- with ResultRelInfos being returned with the incorrect ri_RootResultRelInfo
3122+
WITH cte AS (
3123+
UPDATE fkpart13_t2_p1 SET part_id = part_id
3124+
) UPDATE fkpart13_t1 SET a = 2 WHERE a = 1;
3125+
DROP SCHEMA fkpart13 CASCADE;
3126+
NOTICE: drop cascades to 3 other objects
3127+
DETAIL: drop cascades to table fkpart13_t1
3128+
drop cascades to table fkpart13_t2
3129+
drop cascades to table fkpart13_t3
3130+
RESET search_path;

‎src/test/regress/sql/foreign_key.sql‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2181,3 +2181,51 @@ SET client_min_messages TO warning;
21812181
DROP SCHEMA fkpart12 CASCADE;
21822182
RESET client_min_messages;
21832183
RESET search_path;
2184+
2185+
-- Exercise the column mapping code with foreign keys. In this test we'll
2186+
-- create a partitioned table which has a partition with a dropped column and
2187+
-- check to ensure that an UPDATE cascades the changes correctly to the
2188+
-- partitioned table.
2189+
CREATE SCHEMA fkpart13;
2190+
SET search_path TO fkpart13;
2191+
2192+
CREATE TABLE fkpart13_t1 (a int PRIMARY KEY);
2193+
2194+
CREATE TABLE fkpart13_t2 (
2195+
part_id int PRIMARY KEY,
2196+
column_to_drop int,
2197+
FOREIGN KEY (part_id) REFERENCES fkpart13_t1 ON UPDATE CASCADE ON DELETE CASCADE
2198+
) PARTITION BY LIST (part_id);
2199+
2200+
CREATE TABLE fkpart13_t2_p1 PARTITION OF fkpart13_t2 FOR VALUES IN (1);
2201+
2202+
-- drop the column
2203+
ALTER TABLE fkpart13_t2 DROP COLUMN column_to_drop;
2204+
2205+
-- create a new partition without the dropped column
2206+
CREATE TABLE fkpart13_t2_p2 PARTITION OF fkpart13_t2 FOR VALUES IN (2);
2207+
2208+
CREATE TABLE fkpart13_t3 (
2209+
a int NOT NULL,
2210+
FOREIGN KEY (a)
2211+
REFERENCES fkpart13_t2
2212+
ON UPDATE CASCADE ON DELETE CASCADE
2213+
);
2214+
2215+
INSERT INTO fkpart13_t1 (a) VALUES (1);
2216+
INSERT INTO fkpart13_t2 (part_id) VALUES (1);
2217+
INSERT INTO fkpart13_t3 (a) VALUES (1);
2218+
2219+
-- Test a cascading update works correctly with with the dropped column
2220+
UPDATE fkpart13_t1 SET a = 2 WHERE a = 1;
2221+
SELECT tableoid::regclass,* FROM fkpart13_t2;
2222+
SELECT tableoid::regclass,* FROM fkpart13_t3;
2223+
2224+
-- Exercise code in ExecGetTriggerResultRel() as there's been previous issues
2225+
-- with ResultRelInfos being returned with the incorrect ri_RootResultRelInfo
2226+
WITH cte AS (
2227+
UPDATE fkpart13_t2_p1 SET part_id = part_id
2228+
) UPDATE fkpart13_t1 SET a = 2 WHERE a = 1;
2229+
2230+
DROP SCHEMA fkpart13 CASCADE;
2231+
RESET search_path;

0 commit comments

Comments
 (0)