diff --git a/contrib/test_decoding/expected/toast.out b/contrib/test_decoding/expected/toast.out
index cd03e9d50a1..a757e7dc8d5 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/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml
index c572f75adbf..327aedc409d 100644
--- a/doc/src/sgml/ref/alter_table.sgml
+++ b/doc/src/sgml/ref/alter_table.sgml
@@ -798,10 +798,11 @@ WITH ( MODULUS numeric_literal, REM
This form changes the information which is written to the write-ahead log
- to identify rows which are updated or deleted. This option has no effect
- except when logical replication is in use.
- In all cases, no old values are logged unless at least one of the columns
- that would be logged differs between the old and new versions of the row.
+ to identify rows which are updated or deleted.
+ In most cases, the old value of each column is only logged if it differs
+ from the new value; however, if the old value is stored externally, it is
+ always logged regardless of whether it changed.
+ This option has no effect except when logical replication is in use.
DEFAULT
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 24d39327c99..1107102fcdb 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -76,9 +76,11 @@ static XLogRecPtr log_heap_update(Relation reln, Buffer oldbuf,
Buffer newbuf, HeapTuple oldtup,
HeapTuple newtup, HeapTuple old_key_tup,
bool all_visible_cleared, bool new_all_visible_cleared);
-static Bitmapset *HeapDetermineModifiedColumns(Relation relation,
- Bitmapset *interesting_cols,
- HeapTuple oldtup, HeapTuple newtup);
+static Bitmapset *HeapDetermineColumnsInfo(Relation relation,
+ Bitmapset *interesting_cols,
+ Bitmapset *external_cols,
+ HeapTuple oldtup, HeapTuple newtup,
+ bool *has_external);
static bool heap_acquire_tuplock(Relation relation, ItemPointer tid,
LockTupleMode mode, LockWaitPolicy wait_policy,
bool *have_tuple_lock);
@@ -102,7 +104,7 @@ static void MultiXactIdWait(MultiXactId multi, MultiXactStatus status, uint16 in
static bool ConditionalMultiXactIdWait(MultiXactId multi, MultiXactStatus status,
uint16 infomask, Relation rel, int *remaining);
static XLogRecPtr log_heap_new_cid(Relation relation, HeapTuple tup);
-static HeapTuple ExtractReplicaIdentity(Relation rel, HeapTuple tup, bool key_modified,
+static HeapTuple ExtractReplicaIdentity(Relation rel, HeapTuple tup, bool key_required,
bool *copy);
@@ -2941,6 +2943,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,
@@ -3025,7 +3028,7 @@ heap_update(Relation relation, ItemPointer otid, HeapTuple newtup,
Assert(ItemIdIsNormal(lp));
/*
- * Fill in enough data in oldtup for HeapDetermineModifiedColumns to work
+ * Fill in enough data in oldtup for HeapDetermineColumnsInfo to work
* properly.
*/
oldtup.t_tableOid = RelationGetRelid(relation);
@@ -3036,9 +3039,17 @@ heap_update(Relation relation, ItemPointer otid, HeapTuple newtup,
/* the new tuple is ready, except for this: */
newtup->t_tableOid = RelationGetRelid(relation);
- /* Determine columns modified by the update. */
- modified_attrs = HeapDetermineModifiedColumns(relation, interesting_attrs,
- &oldtup, newtup);
+ /*
+ * Determine columns modified by the update. Additionally, identify
+ * whether any of the unmodified replica identity key attributes in the
+ * old tuple is externally stored or not. This is required because for
+ * such attributes the flattened value won't be WAL logged as part of the
+ * new tuple so we must include it as part of the old_key_tuple. See
+ * ExtractReplicaIdentity.
+ */
+ modified_attrs = HeapDetermineColumnsInfo(relation, interesting_attrs,
+ id_attrs, &oldtup,
+ newtup, &id_has_external);
/*
* If we're not updating any "key" column, we can grab a weaker lock type.
@@ -3639,10 +3650,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 */
@@ -3798,47 +3811,15 @@ l2:
}
/*
- * Check if the specified attribute's value is same in both given tuples.
- * Subroutine for HeapDetermineModifiedColumns.
+ * Check if the specified attribute's values are the same. Subroutine for
+ * HeapDetermineColumnsInfo.
*/
static bool
-heap_tuple_attr_equals(TupleDesc tupdesc, int attrnum,
- HeapTuple tup1, HeapTuple tup2)
+heap_attr_equals(TupleDesc tupdesc, int attrnum, Datum value1, Datum value2,
+ bool isnull1, bool isnull2)
{
- Datum value1,
- value2;
- bool isnull1,
- isnull2;
Form_pg_attribute att;
- /*
- * If it's a whole-tuple reference, say "not equal". It's not really
- * worth supporting this case, since it could only succeed after a no-op
- * update, which is hardly a case worth optimizing for.
- */
- if (attrnum == 0)
- return false;
-
- /*
- * Likewise, automatically say "not equal" for any system attribute other
- * than tableOID; we cannot expect these to be consistent in a HOT chain,
- * or even to be set correctly yet in the new tuple.
- */
- if (attrnum < 0)
- {
- if (attrnum != TableOidAttributeNumber)
- return false;
- }
-
- /*
- * Extract the corresponding values. XXX this is pretty inefficient if
- * there are many indexed columns. Should HeapDetermineModifiedColumns do
- * a single heap_deform_tuple call on each tuple, instead? But that
- * doesn't work for system columns ...
- */
- value1 = heap_getattr(tup1, attrnum, tupdesc, &isnull1);
- value2 = heap_getattr(tup2, attrnum, tupdesc, &isnull2);
-
/*
* If one value is NULL and other is not, then they are certainly not
* equal
@@ -3880,24 +3861,96 @@ heap_tuple_attr_equals(TupleDesc tupdesc, int attrnum,
* Given an updated tuple, determine (and return into the output bitmapset),
* from those listed as interesting, the set of columns that changed.
*
- * The input bitmapset is destructively modified; that is OK since this is
- * invoked at most once in heap_update.
+ * has_external indicates if any of the unmodified attributes (from those
+ * listed as interesting) of the old tuple is a member of external_cols and is
+ * stored externally.
+ *
+ * The input interesting_cols bitmapset is destructively modified; that is OK
+ * since this is invoked at most once in heap_update.
*/
static Bitmapset *
-HeapDetermineModifiedColumns(Relation relation, Bitmapset *interesting_cols,
- HeapTuple oldtup, HeapTuple newtup)
+HeapDetermineColumnsInfo(Relation relation,
+ Bitmapset *interesting_cols,
+ Bitmapset *external_cols,
+ HeapTuple oldtup, HeapTuple newtup,
+ bool *has_external)
{
- int attnum;
+ int attrnum;
Bitmapset *modified = NULL;
+ TupleDesc tupdesc = RelationGetDescr(relation);
- while ((attnum = bms_first_member(interesting_cols)) >= 0)
+ while ((attrnum = bms_first_member(interesting_cols)) >= 0)
{
- attnum += FirstLowInvalidHeapAttributeNumber;
+ Datum value1,
+ value2;
+ bool isnull1,
+ isnull2;
- if (!heap_tuple_attr_equals(RelationGetDescr(relation),
- attnum, oldtup, newtup))
+ attrnum += FirstLowInvalidHeapAttributeNumber;
+
+ /*
+ * If it's a whole-tuple reference, say "not equal". It's not really
+ * worth supporting this case, since it could only succeed after a
+ * no-op update, which is hardly a case worth optimizing for.
+ */
+ if (attrnum == 0)
+ {
modified = bms_add_member(modified,
- attnum - FirstLowInvalidHeapAttributeNumber);
+ attrnum -
+ FirstLowInvalidHeapAttributeNumber);
+ continue;
+ }
+
+ /*
+ * Likewise, automatically say "not equal" for any system attribute
+ * other than tableOID; we cannot expect these to be consistent in a
+ * HOT chain, or even to be set correctly yet in the new tuple.
+ */
+ if (attrnum < 0)
+ {
+ if (attrnum != TableOidAttributeNumber)
+ {
+ modified = bms_add_member(modified,
+ attrnum -
+ FirstLowInvalidHeapAttributeNumber);
+ continue;
+ }
+ }
+
+ /*
+ * Extract the corresponding values. XXX this is pretty inefficient
+ * if there are many indexed columns. Should we do a single
+ * heap_deform_tuple call on each tuple, instead? But that doesn't
+ * work for system columns ...
+ */
+ value1 = heap_getattr(oldtup, attrnum, tupdesc, &isnull1);
+ value2 = heap_getattr(newtup, attrnum, tupdesc, &isnull2);
+
+ if (!heap_attr_equals(tupdesc, attrnum, value1,
+ value2, isnull1, isnull2))
+ {
+ modified = bms_add_member(modified,
+ attrnum -
+ FirstLowInvalidHeapAttributeNumber);
+ continue;
+ }
+
+ /*
+ * No need to check attributes that can't be stored externally. Note
+ * that system attributes can't be stored externally.
+ */
+ if (attrnum < 0 || isnull1 ||
+ TupleDescAttr(tupdesc, attrnum - 1)->attlen != -1)
+ continue;
+
+ /*
+ * Check if the old tuple's attribute is stored externally and is a
+ * member of external_cols.
+ */
+ if (VARATT_IS_EXTERNAL((struct varlena *) DatumGetPointer(value1)) &&
+ bms_is_member(attrnum - FirstLowInvalidHeapAttributeNumber,
+ external_cols))
+ *has_external = true;
}
return modified;
@@ -7661,14 +7714,14 @@ 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.
+ * Pass key_required true if any replica identity columns changed value, or if
+ * any of them have any external data. Delete must always pass true.
*
* *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);
@@ -7700,8 +7753,8 @@ 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 isn't required and we're only logging the key, we're done */
+ if (!key_required)
return NULL;
/* find out the replica identity columns */
@@ -7709,10 +7762,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;