On 10/22/2018 10:00 PM, Amit Langote wrote:

After observing the test case in the provided log, I managed to reproduce
it with the following:

create table foo (a int primary key, b int);
create table bar (a int references foo on delete cascade, b int);
insert into foo values (1, 1);
insert into foo values (2, 2);
alter table foo add c int;
alter table foo drop c;
delete from foo;
server closed the connection unexpectedly
        This probably means the server terminated abnormally
        before or while processing the request.
The connection to the server was lost. Attempting reset: Failed.

Analyzing this crash, I located the bug down to GetTupleForTrigger(), but
perhaps it's really in heap_expand_tuple() / expand_tuple(), where the
value of trigger tuple's t_self is being switched from a valid one to an
invalid value.

In heaptuple.c: expand_tuple()


         ItemPointerSetInvalid(&((*targetHeapTuple)->t_self));


FWIW, attached patch fixes this for me.  Adding Andrew whose recent commit
7636e5c60f [1] seems to have introduced the heap_expan_tuple call in
GetTupleForTrigger.  Maybe, he can better judge a fix for this.




Thanks. I think the line in expand_tuple is a thinko and we should change it, rather than change GetTupleForTrigger().

Here is a patch that does that and also adds your test case to the regression tests.

cheers

andrew

--
Andrew Dunstan                https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

diff --git a/src/backend/access/common/heaptuple.c b/src/backend/access/common/heaptuple.c
index 15444cf..28127b3 100644
--- a/src/backend/access/common/heaptuple.c
+++ b/src/backend/access/common/heaptuple.c
@@ -856,7 +856,7 @@ expand_tuple(HeapTuple *targetHeapTuple,
 			= (HeapTupleHeader) ((char *) *targetHeapTuple + HEAPTUPLESIZE);
 		(*targetHeapTuple)->t_len = len;
 		(*targetHeapTuple)->t_tableOid = sourceTuple->t_tableOid;
-		ItemPointerSetInvalid(&((*targetHeapTuple)->t_self));
+		(*targetHeapTuple)->t_self = sourceTuple->t_self;
 
 		targetTHeader->t_infomask = sourceTHeader->t_infomask;
 		targetTHeader->t_hoff = hoff;
diff --git a/src/test/regress/expected/fast_default.out b/src/test/regress/expected/fast_default.out
index 48bd360..0797e11 100644
--- a/src/test/regress/expected/fast_default.out
+++ b/src/test/regress/expected/fast_default.out
@@ -727,7 +727,17 @@ SELECT * FROM t;
 (1 row)
 
 DROP TABLE t;
+-- make sure expanded tuple has correct self pointer
+-- it will be required by the RI tigger doing the cascading delete
+CREATE TABLE leader (a int PRIMARY KEY, b int);
+CREATE TABLE follower (a int REFERENCES leader ON DELETE CASCADE, b int);
+INSERT INTO leader VALUES (1, 1), (2, 2);
+ALTER TABLE leader ADD c int;
+ALTER TABLE leader DROP c;
+DELETE FROM leader;
 -- cleanup
+DROP TABLE follower;
+DROP TABLE leader;
 DROP FUNCTION test_trigger();
 DROP TABLE t1;
 DROP FUNCTION set(name);
diff --git a/src/test/regress/sql/fast_default.sql b/src/test/regress/sql/fast_default.sql
index 06205cb..eefcd49 100644
--- a/src/test/regress/sql/fast_default.sql
+++ b/src/test/regress/sql/fast_default.sql
@@ -471,7 +471,19 @@ UPDATE t SET y = 2;
 SELECT * FROM t;
 DROP TABLE t;
 
+-- make sure expanded tuple has correct self pointer
+-- it will be required by the RI tigger doing the cascading delete
+
+CREATE TABLE leader (a int PRIMARY KEY, b int);
+CREATE TABLE follower (a int REFERENCES leader ON DELETE CASCADE, b int);
+INSERT INTO leader VALUES (1, 1), (2, 2);
+ALTER TABLE leader ADD c int;
+ALTER TABLE leader DROP c;
+DELETE FROM leader;
+
 -- cleanup
+DROP TABLE follower;
+DROP TABLE leader;
 DROP FUNCTION test_trigger();
 DROP TABLE t1;
 DROP FUNCTION set(name);

Reply via email to