On Wed, Jun 2, 2021 at 7:23 PM Dilip Kumar <dilipbal...@gmail.com> wrote:
>
> On Wed, Jun 2, 2021 at 7:20 PM Kuntal Ghosh <kuntalghosh.2...@gmail.com>
> wrote:
> >
> > On Wed, Jun 2, 2021 at 3:10 PM Dilip Kumar <dilipbal...@gmail.com> wrote:
> > >
> > > Yes, you are right. I will change it in the next version, along with
> > > the test case.
> > >
> > + /*
> > + * 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.
> > + */
> > This doesn't really mention why we need to detoast the key even when
> > the key remains the same. I guess we can add some more details here.
>
> Noted, let's see what others have to say about fixing this, then I
> will fix this along with one other pending comment and I will also add
> the test case. Thanks for looking into this.
I have fixed all the pending issues, I see there is already a test
case for this so I have changed the output for that.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
From fe1b9131c6c53a6335dd4fd7ceb5f8c18aacdba9 Mon Sep 17 00:00:00 2001
From: Dilip Kumar <dilipkumar@localhost.localdomain>
Date: Mon, 31 May 2021 14:37:26 +0530
Subject: [PATCH v3] 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.
---
contrib/test_decoding/expected/toast.out | 2 +-
src/backend/access/heap/heapam.c | 20 ++++++++++++++++++--
2 files changed, 19 insertions(+), 3 deletions(-)
diff --git a/contrib/test_decoding/expected/toast.out b/contrib/test_decoding/expected/toast.out
index 75c4d22..769ab0e 100644
--- a/contrib/test_decoding/expected/toast.out
+++ b/contrib/test_decoding/expected/toast.out
@@ -77,7 +77,7 @@ SELECT substr(data, 1, 200) FROM pg_logical_slot_get_changes('regression_slot',
table public.toasted_key: INSERT: id[integer]:1 toasted_key[text]:'1234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123
COMMIT
BEGIN
- table public.toasted_key: UPDATE: id[integer]:1 toasted_key[text]:unchanged-toast-datum toasted_col1[text]:unchanged-toast-datum toasted_col2[text]:'987654321098765432109876543210987654321098765432109
+ table public.toasted_key: UPDATE: old-key: toasted_key[text]:'123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678
COMMIT
BEGIN
table public.toasted_key: UPDATE: old-key: toasted_key[text]:'123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 2433998..afd775e 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -8339,8 +8339,14 @@ 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 it's possible that key might have
+ * stored externally, if so we need to extract its value because the value
+ * of the externally stored key data will not be logged with the main tuple
+ * so we will have to log as old key tuple for replica identity.
+ */
+ if ((!key_changed) && !HeapTupleHasExternal(tp))
return NULL;
/* find out the replica identity columns */
@@ -8391,6 +8397,16 @@ 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);
+ *copy = false;
+ return NULL;
+ }
return key_tuple;
}
--
1.8.3.1
