From 0b015ef3151eb83817218687234e58c1b265a54c Mon Sep 17 00:00:00 2001 From: David Rowley Date: Tue, 21 Oct 2025 17:03:37 +1300 Subject: [PATCH v4] Fix incorrect logic for caching ResultRelInfos for triggers When dealing with ResultRelInfos for partitions, there are cases where there is 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. The current caching logic only considers the relation's OID when it checks for a pre-built ResultRelInfo. Here we adjust that to also check the ri_RootResultRelInfo matches too. --- src/backend/executor/execMain.c | 23 ++++++--- src/test/regress/expected/foreign_key.out | 59 +++++++++++++++++++++++ src/test/regress/sql/foreign_key.sql | 45 +++++++++++++++++ 3 files changed, 120 insertions(+), 7 deletions(-) diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c index 713e926329c..ac4e4389d6a 100644 --- a/src/backend/executor/execMain.c +++ b/src/backend/executor/execMain.c @@ -1326,10 +1326,8 @@ InitResultRelInfo(ResultRelInfo *resultRelInfo, * Get a ResultRelInfo for a trigger target relation. * * Most of the time, triggers are fired on one of the result relations of the - * query, and so we can just return a member of the es_result_relations array, - * or the es_tuple_routing_result_relations list (if any). (Note: in self-join - * situations there might be multiple members with the same OID; if so it - * doesn't matter which one we pick.) + * query, and so we can just return one we already made and stored in the + * es_opened_result_relations or es_tuple_routing_result_relations Lists. * * However, it is sometimes necessary to fire triggers on other relations; * this happens mainly when an RI update trigger queues additional triggers @@ -1349,11 +1347,20 @@ ExecGetTriggerResultRel(EState *estate, Oid relid, Relation rel; MemoryContext oldcontext; + /* + * Before creating a new ResultRelInfo, check if we've already made and + * cached one for this relation. We must ensure that the given + * 'rootRelInfo' matches the one stored in the cached ResultRelInfo as + * trigger handling for partitions can result in mixed requirements for + * what ri_RootResultRelInfo is set to. + */ + /* Search through the query result relations */ foreach(l, estate->es_opened_result_relations) { rInfo = lfirst(l); - if (RelationGetRelid(rInfo->ri_RelationDesc) == relid) + if (RelationGetRelid(rInfo->ri_RelationDesc) == relid && + rInfo->ri_RootResultRelInfo == rootRelInfo) return rInfo; } @@ -1364,7 +1371,8 @@ ExecGetTriggerResultRel(EState *estate, Oid relid, foreach(l, estate->es_tuple_routing_result_relations) { rInfo = (ResultRelInfo *) lfirst(l); - if (RelationGetRelid(rInfo->ri_RelationDesc) == relid) + if (RelationGetRelid(rInfo->ri_RelationDesc) == relid && + rInfo->ri_RootResultRelInfo == rootRelInfo) return rInfo; } @@ -1372,7 +1380,8 @@ ExecGetTriggerResultRel(EState *estate, Oid relid, foreach(l, estate->es_trig_target_relations) { rInfo = (ResultRelInfo *) lfirst(l); - if (RelationGetRelid(rInfo->ri_RelationDesc) == relid) + if (RelationGetRelid(rInfo->ri_RelationDesc) == relid && + rInfo->ri_RootResultRelInfo == rootRelInfo) return rInfo; } /* Nope, so we need a new one */ diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out index dc541d61adf..2d630d118ca 100644 --- a/src/test/regress/expected/foreign_key.out +++ b/src/test/regress/expected/foreign_key.out @@ -3406,3 +3406,62 @@ SET client_min_messages TO warning; DROP SCHEMA fkpart12 CASCADE; RESET client_min_messages; RESET search_path; +-- Exercise the column mapping code with foreign keys. In this test we'll +-- create a partitioned table which has a partition with a dropped column and +-- check to ensure that an UPDATE cascades the changes correctly to the +-- partitioned table. +CREATE SCHEMA fkpart13; +SET search_path TO fkpart13; +CREATE TABLE top_tbl ( + id int PRIMARY KEY +); +CREATE TABLE middle_tbl_parted ( + partition_id int PRIMARY KEY, + column_to_drop int, + FOREIGN KEY (partition_id) REFERENCES top_tbl (id) ON UPDATE CASCADE ON DELETE CASCADE +) PARTITION BY LIST (partition_id); +CREATE TABLE middle_tbl_parted_1 PARTITION OF middle_tbl_parted FOR VALUES IN (1); +-- drop the column +ALTER TABLE middle_tbl_parted DROP COLUMN column_to_drop; +-- create a new partition without the dropped column +CREATE TABLE middle_tbl_parted_2 PARTITION OF middle_tbl_parted FOR VALUES IN (2); +CREATE TABLE referencing_tbl ( + id int NOT NULL, + FOREIGN KEY (id) + REFERENCES middle_tbl_parted(partition_id) + ON UPDATE CASCADE ON DELETE CASCADE +); +INSERT INTO top_tbl (id) VALUES (1); +INSERT INTO middle_tbl_parted (partition_id) VALUES (1); +INSERT INTO referencing_tbl (id) VALUES (1); +-- Test a cascading update works correctly with with the dropped column +SELECT tableoid::regclass,* FROM middle_tbl_parted; + tableoid | partition_id +---------------------+-------------- + middle_tbl_parted_1 | 1 +(1 row) + +SELECT tableoid::regclass,* FROM referencing_tbl; + tableoid | id +-----------------+---- + referencing_tbl | 1 +(1 row) + +UPDATE top_tbl SET id = 2 WHERE id = 1; +SELECT tableoid::regclass,* FROM middle_tbl_parted; + tableoid | partition_id +---------------------+-------------- + middle_tbl_parted_2 | 2 +(1 row) + +SELECT tableoid::regclass,* FROM referencing_tbl; + tableoid | id +-----------------+---- + referencing_tbl | 2 +(1 row) + +DROP SCHEMA fkpart13 CASCADE; +NOTICE: drop cascades to 3 other objects +DETAIL: drop cascades to table top_tbl +drop cascades to table middle_tbl_parted +drop cascades to table referencing_tbl diff --git a/src/test/regress/sql/foreign_key.sql b/src/test/regress/sql/foreign_key.sql index 39174ad1eb9..18a6b014c8a 100644 --- a/src/test/regress/sql/foreign_key.sql +++ b/src/test/regress/sql/foreign_key.sql @@ -2386,3 +2386,48 @@ SET client_min_messages TO warning; DROP SCHEMA fkpart12 CASCADE; RESET client_min_messages; RESET search_path; + +-- Exercise the column mapping code with foreign keys. In this test we'll +-- create a partitioned table which has a partition with a dropped column and +-- check to ensure that an UPDATE cascades the changes correctly to the +-- partitioned table. +CREATE SCHEMA fkpart13; +SET search_path TO fkpart13; + +CREATE TABLE top_tbl ( + id int PRIMARY KEY +); + +CREATE TABLE middle_tbl_parted ( + partition_id int PRIMARY KEY, + column_to_drop int, + FOREIGN KEY (partition_id) REFERENCES top_tbl (id) ON UPDATE CASCADE ON DELETE CASCADE +) PARTITION BY LIST (partition_id); + +CREATE TABLE middle_tbl_parted_1 PARTITION OF middle_tbl_parted FOR VALUES IN (1); + +-- drop the column +ALTER TABLE middle_tbl_parted DROP COLUMN column_to_drop; + +-- create a new partition without the dropped column +CREATE TABLE middle_tbl_parted_2 PARTITION OF middle_tbl_parted FOR VALUES IN (2); + +CREATE TABLE referencing_tbl ( + id int NOT NULL, + FOREIGN KEY (id) + REFERENCES middle_tbl_parted(partition_id) + ON UPDATE CASCADE ON DELETE CASCADE +); + +INSERT INTO top_tbl (id) VALUES (1); +INSERT INTO middle_tbl_parted (partition_id) VALUES (1); +INSERT INTO referencing_tbl (id) VALUES (1); + +-- Test a cascading update works correctly with with the dropped column +SELECT tableoid::regclass,* FROM middle_tbl_parted; +SELECT tableoid::regclass,* FROM referencing_tbl; +UPDATE top_tbl SET id = 2 WHERE id = 1; +SELECT tableoid::regclass,* FROM middle_tbl_parted; +SELECT tableoid::regclass,* FROM referencing_tbl; + +DROP SCHEMA fkpart13 CASCADE; -- 2.43.0