On Tue, Jun 1, 2021 at 3:39 PM Dilip Kumar <dilipbal...@gmail.com> wrote:
>
> On Tue, Jun 1, 2021 at 12:29 PM tanghy.f...@fujitsu.com

> > I noticed that in case1, ExtractReplicaIdentity function returned NULL on 
> > HEAD. But after your fix, it didn’t return NULL. There is no problem with 
> > this case on HEAD, but the patch modified its return value. I’m not sure if 
> > it would bring new problems. Have you checked it?
>
> Good observation, basically, my check says that any field in the tuple
> is toasted then prepare the key tuple, actually, after that, I should
> recheck whether the key field specifically toasted or not and if it is
> not then we can continue returning NULL.  I will fix this in the next
> version.

Attached patch fixes that, I haven't yet added the test case.  Once
someone confirms on the approach then I will add a test case to the
patch.

-- 
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
From 3be161266dd013b8fdd68352dc1c6bd34ed36069 Mon Sep 17 00:00:00 2001
From: Dilip Kumar <dilipkumar@localhost.localdomain>
Date: Mon, 31 May 2021 14:37:26 +0530
Subject: [PATCH v2] Extract unchanged replica identity key if it is stored
 externally

If replica identity is set to key and the key is not modified we don't log
key seperately because it should be logged along with the updated tuple.  But
if the key is stored externally we must have to detoast and log it separately.
---
 src/backend/access/heap/heapam.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index bd60129..9b53cdc 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -8407,8 +8407,11 @@ ExtractReplicaIdentity(Relation relation, HeapTuple tp, bool key_changed,
 		return tp;
 	}
 
-	/* if the key hasn't changed and we're only logging the key, we're done */
-	if (!key_changed)
+	/*
+	 * if the key hasn't changed and we're only logging the key, we're done.
+	 * But if tuple has external data then we might have to detoast the key.
+	 */
+	if ((!key_changed) && !HeapTupleHasExternal(tp))
 		return NULL;
 
 	/* find out the replica identity columns */
@@ -8459,6 +8462,15 @@ ExtractReplicaIdentity(Relation relation, HeapTuple tp, bool key_changed,
 		key_tuple = toast_flatten_tuple(oldtup, desc);
 		heap_freetuple(oldtup);
 	}
+	/*
+	 * If key tuple doesn't have any external data and key is not changed then
+	 * just free the key tuple and return NULL.
+	 */
+	else if (!key_changed)
+	{
+		heap_freetuple(key_tuple);
+		return NULL;
+	}
 
 	return key_tuple;
 }
-- 
1.8.3.1

Reply via email to