diff --git a/contrib/test_decoding/expected/toast.out b/contrib/test_decoding/expected/toast.out
index cd03e9d50a..a757e7dc8d 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 3e18b7dba5..8564adb3d2 100644
--- a/doc/src/sgml/ref/alter_table.sgml
+++ b/doc/src/sgml/ref/alter_table.sgml
@@ -813,10 +813,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 834435cdf7..0735319842 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -101,9 +101,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);
@@ -127,7 +129,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);
static bool ProjIndexIsUnchanged(Relation relation, HeapTuple oldtup, HeapTuple newtup);
@@ -3568,6 +3570,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,
@@ -3652,7 +3655,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);
@@ -3678,9 +3681,17 @@ heap_update(Relation relation, ItemPointer otid, HeapTuple newtup,
Assert(!(newtup->t_data->t_infomask & HEAP_HASOID));
}
- /* 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.
@@ -4266,10 +4277,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 */
@@ -4426,48 +4439,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 OID and 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 != ObjectIdAttributeNumber &&
- 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
@@ -4589,24 +4569,97 @@ ProjIndexIsUnchanged(Relation relation, HeapTuple oldtup, HeapTuple newtup)
* 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 OID and 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 != ObjectIdAttributeNumber &&
+ 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;
@@ -8116,9 +8169,12 @@ 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 in the Relation relation.
+ *
+ * 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.
*/
static HeapTuple
-ExtractReplicaIdentity(Relation relation, HeapTuple tp, bool key_changed, bool *copy)
+ExtractReplicaIdentity(Relation relation, HeapTuple tp, bool key_required, bool *copy)
{
TupleDesc desc = RelationGetDescr(relation);
Oid replidindex;
@@ -8151,8 +8207,8 @@ ExtractReplicaIdentity(Relation relation, HeapTuple tp, bool key_changed, bool *
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 the replica identity index */