v3-0001-Fix-incorrect-reuse-of-ResultRelInfo-in-trigger-e.patch

application/octet-stream

Filename: v3-0001-Fix-incorrect-reuse-of-ResultRelInfo-in-trigger-e.patch
Type: application/octet-stream
Part: 0
Message: Re: Segfault in RI UPDATE CASCADE on partitioned tables with LIKE+ATTACH child (attnum drift)

Patch

Same data as JSON: GET /api/v1/attachments/:id/patch the parsed metadata as JSON — format, series position, per-file stats; never the diff bytes. API reference →
Format: format-patch
Series: patch v3-0001
Subject: Fix incorrect reuse of ResultRelInfo in trigger executor path
File+
src/backend/executor/execMain.c 18 1
src/test/regress/expected/foreign_key.out 79 0
src/test/regress/sql/foreign_key.sql 79 0
From 52564baef7b6d6b90e0a5d5af931528aea690ead Mon Sep 17 00:00:00 2001
From: Amit Langote <amitlan@postgresql.org>
Date: Mon, 20 Oct 2025 22:12:55 +0900
Subject: [PATCH v3] Fix incorrect reuse of ResultRelInfo in trigger executor
 path

When a partitioned table UPDATE cascades via a trigger, it may happen
that ExecGetTriggerResultRel() returns a cached ResultRelInfo whose
ri_RootResultRelInfo is NULL because it was created earlier by
afterTriggerInvokeEvents() with a NULL rootRelInfo.  That causes
ExecGetChildToRootMap() to skip tuple translation, leading to wrong
behaviour in partitioned UPDATE cascades.

Add an assertion to verify that when returning a cached ResultRelInfo,
the rootRelInfo matches or the requested one is NULL.  This prevents
reusing a cached ResultRelInfo with a NULL rootRelInfo in contexts
where a valid one is expected.

Reported-by: Dmitry Fomin <fomin.list@gmail.com>
Diagnozed-by: David Rowley <dgrowleyml@gmail.com>
Author: David Rowley <dgrowleyml@gmail.com>
Co-authored-by: Amit Langote <amitlangote09@gmail.com>
Discussion: https://postgr.es/m/7DCE78D7-0520-4207-822B-92F60AEA14B4@gmail.com
---
 src/backend/executor/execMain.c           | 19 +++++-
 src/test/regress/expected/foreign_key.out | 79 +++++++++++++++++++++++
 src/test/regress/sql/foreign_key.sql      | 79 +++++++++++++++++++++++
 3 files changed, 176 insertions(+), 1 deletion(-)

diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index 831c55ce787..d648a74790d 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -1339,6 +1339,13 @@ InitResultRelInfo(ResultRelInfo *resultRelInfo,
  * also provides a way for EXPLAIN ANALYZE to report the runtimes of such
  * triggers.)  So we make additional ResultRelInfo's as needed, and save them
  * in es_trig_target_relations.
+ *
+ * When matching cached entries, we normally allow a caller that passes
+ * NULL for rootRelInfo to reuse any matching entry, but entries in
+ * es_trig_target_relations require an exact root match.  That list may
+ * hold entries created in different contexts, including rootless ones
+ * from the INSERT side of a cross-partition update, and loose matching
+ * there could skip required tuple translation.
  */
 ResultRelInfo *
 ExecGetTriggerResultRel(EState *estate, Oid relid,
@@ -1354,7 +1361,12 @@ ExecGetTriggerResultRel(EState *estate, Oid relid,
 	{
 		rInfo = lfirst(l);
 		if (RelationGetRelid(rInfo->ri_RelationDesc) == relid)
+		{
+			Assert(rInfo->ri_RootResultRelInfo == rootRelInfo ||
+				   rootRelInfo == NULL);
 			return rInfo;
+
+		}
 	}
 
 	/*
@@ -1365,14 +1377,19 @@ ExecGetTriggerResultRel(EState *estate, Oid relid,
 	{
 		rInfo = (ResultRelInfo *) lfirst(l);
 		if (RelationGetRelid(rInfo->ri_RelationDesc) == relid)
+		{
+			Assert(rInfo->ri_RootResultRelInfo == rootRelInfo ||
+				   rootRelInfo == NULL);
 			return rInfo;
+		}
 	}
 
 	/* Nope, but maybe we already made an extra ResultRelInfo for it */
 	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..b6e64fae45d 100644
--- a/src/test/regress/expected/foreign_key.out
+++ b/src/test/regress/expected/foreign_key.out
@@ -3406,3 +3406,82 @@ SET client_min_messages TO warning;
 DROP SCHEMA fkpart12 CASCADE;
 RESET client_min_messages;
 RESET search_path;
+-- Test for bug where cascading UPDATE on partitioned tables could fail
+-- due to cached ResultRelInfo lacking ri_RootResultRelInfo.
+--
+-- The problem occurred when ExecGetTriggerResultRel() reused a
+-- ResultRelInfo created earlier by afterTriggerInvokeEvents() with
+-- a NULL rootRelInfo.  On a subsequent call, the cached entry was
+-- returned instead of building one with the correct rootRelInfo.
+-- This caused ExecGetChildToRootMap() to skip tuple translation,
+-- leading to incorrect tuple formats or crashes during ON UPDATE
+-- CASCADE between partitioned tables.
+--
+-- https://postgr.es/m/7DCE78D7-0520-4207-822B-92F60AEA14B4@gmail.com
+-- Parent partitioned table
+CREATE TABLE parted_cp_update_bug_1 (a int, b int) PARTITION BY LIST (a);
+CREATE TABLE parted_cp_update_bug_1_p1 PARTITION OF parted_cp_update_bug_1 FOR VALUES IN (1);
+CREATE TABLE parted_cp_update_bug_1_p2 PARTITION OF parted_cp_update_bug_1 FOR VALUES IN (2);
+-- Child 1. Partitioned. One child is attached after column surgery to
+-- mirror Dmitry's case where the ResultRelInfo cache path is exercised.
+CREATE TABLE parted_cp_update_bug_2 (a int, b int) PARTITION BY LIST (a);
+CREATE TABLE parted_cp_update_bug_2_p1 PARTITION OF parted_cp_update_bug_2 FOR VALUES IN (1);
+CREATE TABLE parted_cp_update_bug_2_p2 (a int, c int, b int);
+ALTER TABLE parted_cp_update_bug_2_p2 DROP COLUMN c;
+ALTER TABLE parted_cp_update_bug_2
+  ATTACH PARTITION parted_cp_update_bug_2_p2 FOR VALUES IN (2);
+-- Child 2. Partitioned. Will reference both parent and child1.
+CREATE TABLE parted_cp_update_bug_3 (a int, b int) PARTITION BY LIST (a);
+CREATE TABLE parted_cp_update_bug_3_p1 PARTITION OF parted_cp_update_bug_3 FOR VALUES IN (1);
+CREATE TABLE parted_cp_update_bug_3_p2 PARTITION OF parted_cp_update_bug_3 FOR VALUES IN (2);
+-- Keys and FKs
+ALTER TABLE parted_cp_update_bug_1 ADD PRIMARY KEY (a);
+ALTER TABLE parted_cp_update_bug_2 ADD PRIMARY KEY (a);
+-- FK with ON UPDATE CASCADE from child1 to parent
+ALTER TABLE parted_cp_update_bug_2
+  ADD FOREIGN KEY (a) REFERENCES parted_cp_update_bug_1 ON UPDATE CASCADE;
+-- Two FKs on child2:
+--   1) to parent with ON UPDATE CASCADE
+--   2) to child1 without cascade (to match the original setup)
+ALTER TABLE parted_cp_update_bug_3
+  ADD FOREIGN KEY (a) REFERENCES parted_cp_update_bug_1 ON UPDATE CASCADE;
+ALTER TABLE parted_cp_update_bug_3
+  ADD FOREIGN KEY (a) REFERENCES parted_cp_update_bug_2;
+-- Seed rows go to the a=1 partitions
+INSERT INTO parted_cp_update_bug_1 VALUES (1, 1);
+INSERT INTO parted_cp_update_bug_2 VALUES (1, 1);
+INSERT INTO parted_cp_update_bug_3 VALUES (1, 1);
+-- Cascading update on parent. Before the fix this can take the
+-- cached ResultRelInfo without ri_RootResultRelInfo and fail to
+-- translate the tuple for the partitioned root format.
+UPDATE parted_cp_update_bug_1 SET a = 2 WHERE a = 1;
+-- Verify that each row moved to its a=2 partition and values are intact
+SELECT tableoid::regclass AS rel, a, b
+FROM parted_cp_update_bug_1
+ORDER BY a;
+            rel            | a | b 
+---------------------------+---+---
+ parted_cp_update_bug_1_p2 | 2 | 1
+(1 row)
+
+SELECT tableoid::regclass AS rel, a, b
+FROM parted_cp_update_bug_2
+ORDER BY a;
+            rel            | a | b 
+---------------------------+---+---
+ parted_cp_update_bug_2_p2 | 2 | 1
+(1 row)
+
+SELECT tableoid::regclass AS rel, a, b
+FROM parted_cp_update_bug_3
+ORDER BY a;
+            rel            | a | b 
+---------------------------+---+---
+ parted_cp_update_bug_3_p2 | 2 | 1
+(1 row)
+
+-- Clean up
+DROP TABLE parted_cp_update_bug_3 CASCADE;
+DROP TABLE parted_cp_update_bug_2 CASCADE;
+DROP TABLE parted_cp_update_bug_1 CASCADE;
+RESET client_min_messages;
diff --git a/src/test/regress/sql/foreign_key.sql b/src/test/regress/sql/foreign_key.sql
index 39174ad1eb9..eaa320f2372 100644
--- a/src/test/regress/sql/foreign_key.sql
+++ b/src/test/regress/sql/foreign_key.sql
@@ -2386,3 +2386,82 @@ SET client_min_messages TO warning;
 DROP SCHEMA fkpart12 CASCADE;
 RESET client_min_messages;
 RESET search_path;
+
+-- Test for bug where cascading UPDATE on partitioned tables could fail
+-- due to cached ResultRelInfo lacking ri_RootResultRelInfo.
+--
+-- The problem occurred when ExecGetTriggerResultRel() reused a
+-- ResultRelInfo created earlier by afterTriggerInvokeEvents() with
+-- a NULL rootRelInfo.  On a subsequent call, the cached entry was
+-- returned instead of building one with the correct rootRelInfo.
+-- This caused ExecGetChildToRootMap() to skip tuple translation,
+-- leading to incorrect tuple formats or crashes during ON UPDATE
+-- CASCADE between partitioned tables.
+--
+-- https://postgr.es/m/7DCE78D7-0520-4207-822B-92F60AEA14B4@gmail.com
+
+
+-- Parent partitioned table
+CREATE TABLE parted_cp_update_bug_1 (a int, b int) PARTITION BY LIST (a);
+CREATE TABLE parted_cp_update_bug_1_p1 PARTITION OF parted_cp_update_bug_1 FOR VALUES IN (1);
+CREATE TABLE parted_cp_update_bug_1_p2 PARTITION OF parted_cp_update_bug_1 FOR VALUES IN (2);
+
+-- Child 1. Partitioned. One child is attached after column surgery to
+-- mirror Dmitry's case where the ResultRelInfo cache path is exercised.
+CREATE TABLE parted_cp_update_bug_2 (a int, b int) PARTITION BY LIST (a);
+CREATE TABLE parted_cp_update_bug_2_p1 PARTITION OF parted_cp_update_bug_2 FOR VALUES IN (1);
+CREATE TABLE parted_cp_update_bug_2_p2 (a int, c int, b int);
+ALTER TABLE parted_cp_update_bug_2_p2 DROP COLUMN c;
+ALTER TABLE parted_cp_update_bug_2
+  ATTACH PARTITION parted_cp_update_bug_2_p2 FOR VALUES IN (2);
+
+-- Child 2. Partitioned. Will reference both parent and child1.
+CREATE TABLE parted_cp_update_bug_3 (a int, b int) PARTITION BY LIST (a);
+CREATE TABLE parted_cp_update_bug_3_p1 PARTITION OF parted_cp_update_bug_3 FOR VALUES IN (1);
+CREATE TABLE parted_cp_update_bug_3_p2 PARTITION OF parted_cp_update_bug_3 FOR VALUES IN (2);
+
+-- Keys and FKs
+ALTER TABLE parted_cp_update_bug_1 ADD PRIMARY KEY (a);
+ALTER TABLE parted_cp_update_bug_2 ADD PRIMARY KEY (a);
+
+-- FK with ON UPDATE CASCADE from child1 to parent
+ALTER TABLE parted_cp_update_bug_2
+  ADD FOREIGN KEY (a) REFERENCES parted_cp_update_bug_1 ON UPDATE CASCADE;
+
+-- Two FKs on child2:
+--   1) to parent with ON UPDATE CASCADE
+--   2) to child1 without cascade (to match the original setup)
+ALTER TABLE parted_cp_update_bug_3
+  ADD FOREIGN KEY (a) REFERENCES parted_cp_update_bug_1 ON UPDATE CASCADE;
+ALTER TABLE parted_cp_update_bug_3
+  ADD FOREIGN KEY (a) REFERENCES parted_cp_update_bug_2;
+
+-- Seed rows go to the a=1 partitions
+INSERT INTO parted_cp_update_bug_1 VALUES (1, 1);
+INSERT INTO parted_cp_update_bug_2 VALUES (1, 1);
+INSERT INTO parted_cp_update_bug_3 VALUES (1, 1);
+
+-- Cascading update on parent. Before the fix this can take the
+-- cached ResultRelInfo without ri_RootResultRelInfo and fail to
+-- translate the tuple for the partitioned root format.
+UPDATE parted_cp_update_bug_1 SET a = 2 WHERE a = 1;
+
+-- Verify that each row moved to its a=2 partition and values are intact
+SELECT tableoid::regclass AS rel, a, b
+FROM parted_cp_update_bug_1
+ORDER BY a;
+
+SELECT tableoid::regclass AS rel, a, b
+FROM parted_cp_update_bug_2
+ORDER BY a;
+
+SELECT tableoid::regclass AS rel, a, b
+FROM parted_cp_update_bug_3
+ORDER BY a;
+
+-- Clean up
+DROP TABLE parted_cp_update_bug_3 CASCADE;
+DROP TABLE parted_cp_update_bug_2 CASCADE;
+DROP TABLE parted_cp_update_bug_1 CASCADE;
+
+RESET client_min_messages;
-- 
2.47.3