v4-0001-Extract-unchanged-external-replica-identity-key.patch

application/x-patch

Filename: v4-0001-Extract-unchanged-external-replica-identity-key.patch
Type: application/x-patch
Part: 0
Message: Re: [BUG]Update Toast data failure in logical replication

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 v4-0001
Subject: Extract unchanged external replica identity key
File+
contrib/test_decoding/expected/toast.out 1 1
src/backend/access/heap/heapam.c 36 15
From 3879809bf1a995995dccf5064eb9282f72af0412 Mon Sep 17 00:00:00 2001
From: Dilip Kumar <dilipkumar@localhost.localdomain>
Date: Wed, 11 Aug 2021 18:09:25 +0530
Subject: [PATCH v4] Extract unchanged external replica identity key

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         | 51 ++++++++++++++++++++++----------
 2 files changed, 37 insertions(+), 16 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..e788926 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -80,7 +80,9 @@ static XLogRecPtr log_heap_update(Relation reln, Buffer oldbuf,
 								  bool all_visible_cleared, bool new_all_visible_cleared);
 static Bitmapset *HeapDetermineModifiedColumns(Relation relation,
 											   Bitmapset *interesting_cols,
-											   HeapTuple oldtup, HeapTuple newtup);
+											   Bitmapset *check_external_attr,
+											   HeapTuple oldtup, HeapTuple newtup,
+											   bool *id_has_external);
 static bool heap_acquire_tuplock(Relation relation, ItemPointer tid,
 								 LockTupleMode mode, LockWaitPolicy wait_policy,
 								 bool *have_tuple_lock);
@@ -106,7 +108,7 @@ static bool ConditionalMultiXactIdWait(MultiXactId multi, MultiXactStatus status
 static void index_delete_sort(TM_IndexDeleteOp *delstate);
 static int	bottomup_sort_and_shrink(TM_IndexDeleteOp *delstate);
 static XLogRecPtr log_heap_new_cid(Relation relation, HeapTuple tup);
-static HeapTuple ExtractReplicaIdentity(Relation rel, HeapTuple tup, bool key_changed,
+static HeapTuple ExtractReplicaIdentity(Relation rel, HeapTuple tup, bool key_required,
 										bool *copy);
 
 
@@ -3185,6 +3187,7 @@ heap_update(Relation relation, ItemPointer otid, HeapTuple newtup,
 	bool		all_visible_cleared_new = false;
 	bool		checked_lockers;
 	bool		locker_remains;
+	bool		id_has_external = false;
 	TransactionId xmax_new_tuple,
 				xmax_old_tuple;
 	uint16		infomask_old_tuple,
@@ -3282,7 +3285,8 @@ heap_update(Relation relation, ItemPointer otid, HeapTuple newtup,
 
 	/* Determine columns modified by the update. */
 	modified_attrs = HeapDetermineModifiedColumns(relation, interesting_attrs,
-												  &oldtup, newtup);
+												  id_attrs, &oldtup,
+												  newtup, &id_has_external);
 
 	/*
 	 * If we're not updating any "key" column, we can grab a weaker lock type.
@@ -3883,10 +3887,12 @@ l2:
 	 * Compute replica identity tuple before entering the critical section so
 	 * we don't PANIC upon a memory allocation failure.
 	 * ExtractReplicaIdentity() will return NULL if nothing needs to be
-	 * logged.
+	 * logged.  Pass old key required as true only if the replica identity key
+	 * columns are modified or it has external data.
 	 */
 	old_key_tuple = ExtractReplicaIdentity(relation, &oldtup,
-										   bms_overlap(modified_attrs, id_attrs),
+										   bms_overlap(modified_attrs, id_attrs) ||
+										   id_has_external,
 										   &old_key_copied);
 
 	/* NO EREPORT(ERROR) from here till changes are logged */
@@ -4047,7 +4053,8 @@ l2:
  */
 static bool
 heap_tuple_attr_equals(TupleDesc tupdesc, int attrnum,
-					   HeapTuple tup1, HeapTuple tup2)
+					   HeapTuple tup1, HeapTuple tup2,
+					   bool *old_has_external)
 {
 	Datum		value1,
 				value2;
@@ -4083,6 +4090,13 @@ heap_tuple_attr_equals(TupleDesc tupdesc, int attrnum,
 	value1 = heap_getattr(tup1, attrnum, tupdesc, &isnull1);
 	value2 = heap_getattr(tup2, attrnum, tupdesc, &isnull2);
 
+	/* check whether the value is stored externally or not and set the flag */
+	if (!isnull1 && TupleDescAttr(tupdesc, attrnum - 1)->attlen == -1 &&
+		VARATT_IS_EXTERNAL((struct varlena *) DatumGetPointer(value1)))
+		*old_has_external = true;
+	else
+		*old_has_external = false;
+
 	/*
 	 * If one value is NULL and other is not, then they are certainly not
 	 * equal
@@ -4129,19 +4143,25 @@ heap_tuple_attr_equals(TupleDesc tupdesc, int attrnum,
  */
 static Bitmapset *
 HeapDetermineModifiedColumns(Relation relation, Bitmapset *interesting_cols,
-							 HeapTuple oldtup, HeapTuple newtup)
+							 Bitmapset *check_external_attr,
+							 HeapTuple oldtup, HeapTuple newtup,
+							 bool *id_has_external)
 {
 	int			attnum;
 	Bitmapset  *modified = NULL;
+	bool		has_external = false;
 
 	while ((attnum = bms_first_member(interesting_cols)) >= 0)
 	{
 		attnum += FirstLowInvalidHeapAttributeNumber;
 
 		if (!heap_tuple_attr_equals(RelationGetDescr(relation),
-									attnum, oldtup, newtup))
+									attnum, oldtup, newtup, &has_external))
 			modified = bms_add_member(modified,
 									  attnum - FirstLowInvalidHeapAttributeNumber);
+		if (has_external && bms_is_member(attnum - FirstLowInvalidHeapAttributeNumber,
+										  check_external_attr))
+			*id_has_external = true;
 	}
 
 	return modified;
@@ -8300,14 +8320,15 @@ log_heap_new_cid(Relation relation, HeapTuple tup)
  * Returns NULL if there's no need to log an identity or if there's no suitable
  * key defined.
  *
- * key_changed should be false if caller knows that no replica identity
- * columns changed value.  It's always true in the DELETE case.
+ * key_required should be false if caller knows that no replica identity
+ * columns changed value and it doesn't has any external data.
+ * It's always true in the DELETE case.
  *
  * *copy is set to true if the returned tuple is a modified copy rather than
  * the same tuple that was passed in.
  */
 static HeapTuple
-ExtractReplicaIdentity(Relation relation, HeapTuple tp, bool key_changed,
+ExtractReplicaIdentity(Relation relation, HeapTuple tp, bool key_required,
 					   bool *copy)
 {
 	TupleDesc	desc = RelationGetDescr(relation);
@@ -8340,7 +8361,7 @@ ExtractReplicaIdentity(Relation relation, HeapTuple tp, bool key_changed,
 	}
 
 	/* if the key hasn't changed and we're only logging the key, we're done */
-	if (!key_changed)
+	if (!key_required)
 		return NULL;
 
 	/* find out the replica identity columns */
@@ -8348,10 +8369,10 @@ ExtractReplicaIdentity(Relation relation, HeapTuple tp, bool key_changed,
 										 INDEX_ATTR_BITMAP_IDENTITY_KEY);
 
 	/*
-	 * If there's no defined replica identity columns, treat as !key_changed.
+	 * If there's no defined replica identity columns, treat as !key_required.
 	 * (This case should not be reachable from heap_update, since that should
-	 * calculate key_changed accurately.  But heap_delete just passes constant
-	 * true for key_changed, so we can hit this case in deletes.)
+	 * calculate key_required accurately.  But heap_delete just passes constant
+	 * true for key_required, so we can hit this case in deletes.)
 	 */
 	if (bms_is_empty(idattrs))
 		return NULL;
-- 
1.8.3.1