rdelaage
/
postgresql
mirror of https://git.postgresql.org/git/postgresql.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

WAL log unchanged toasted replica identity key attributes. 

Currently, during UPDATE, the unchanged replica identity key attributes
are not logged separately because they are getting logged as part of the
new tuple. But if they are stored externally then the untoasted values are
not getting logged as part of the new tuple and logical replication won't
be able to replicate such UPDATEs. So we need to log such attributes as
part of the old_key_tuple during UPDATE.

Reported-by: Haiying Tang
Author: Dilip Kumar and Amit Kapila
Reviewed-by: Alvaro Herrera, Haiying Tang, Andres Freund
Backpatch-through: 10
Discussion: https://postgr.es/m/OS0PR01MB611342D0A92D4F4BF26C0F47FB229@OS0PR01MB6113.jpnprd01.prod.outlook.com
This commit is contained in:
Amit Kapila 2022-02-14 08:55:58 +05:30
parent 0052fb4890
commit 5e01001ffb
3 changed files with 123 additions and 69 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
contrib/test_decoding/expected/toast.out
View File

@ -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

9
doc/src/sgml/ref/alter_table.sgml
View File

@ -871,10 +871,11 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
<listitem>
<para>
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.
<variablelist>
<varlistentry>
<term><literal>DEFAULT</literal></term>

181
src/backend/access/heap/heapam.c
View File

@ -78,9 +78,11 @@ static XLogRecPtr log_heap_update(Relation reln, Buffer oldbuf,
Buffer newbuf, HeapTuple oldtup,
HeapTuple newtup, HeapTuple old_key_tuple,
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);
@ -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);
@ -3184,6 +3186,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,
@ -3251,7 +3254,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);
@ -3262,9 +3265,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.
@ -3864,10 +3875,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 */
@ -4023,47 +4036,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
@ -4105,24 +4086,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;
@ -8359,14 +8412,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);
@ -8398,8 +8451,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 */
@ -8407,10 +8460,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;