From ac9ed2f7d99dd157032efb17889e77b182aee6e3 Mon Sep 17 00:00:00 2001 From: Aleksander Alekseev Date: Wed, 26 Jul 2023 16:15:38 +0300 Subject: [PATCH v4] Remove ReorderBufferTupleBuf structure The fields 'node' and 'alloc_tuple_size' of ReorderBufferTupleBuf structure are no longer used, which leaves only 'tuple' field. Since there is little sense to keep a one-field structure, remove ReorderBufferTupleBuf altogether and refactor the code accordingly. Not backpatched because of ABI change. Aleksander Alekseev, based on the report and patch by Masahiko Sawada. Reviewed by Amit Kapila. Discussion: https://postgr.es/m/CAD21AoCvnuxiXXfRecp7g9+CeC35POQfhuQeJFr7_9u_Q5jc_Q@mail.gmail.com --- contrib/test_decoding/test_decoding.c | 8 +- src/backend/replication/logical/decode.c | 28 +++--- .../replication/logical/reorderbuffer.c | 92 +++++++++---------- src/backend/replication/pgoutput/pgoutput.c | 4 +- src/include/replication/reorderbuffer.h | 39 +++----- src/tools/pgindent/typedefs.list | 1 - 6 files changed, 75 insertions(+), 97 deletions(-) diff --git a/contrib/test_decoding/test_decoding.c b/contrib/test_decoding/test_decoding.c index 4f4f51a89c..7c50d13969 100644 --- a/contrib/test_decoding/test_decoding.c +++ b/contrib/test_decoding/test_decoding.c @@ -640,7 +640,7 @@ pg_decode_change(LogicalDecodingContext *ctx, ReorderBufferTXN *txn, appendStringInfoString(ctx->out, " (no-tuple-data)"); else tuple_to_stringinfo(ctx->out, tupdesc, - &change->data.tp.newtuple->tuple, + change->data.tp.newtuple, false); break; case REORDER_BUFFER_CHANGE_UPDATE: @@ -649,7 +649,7 @@ pg_decode_change(LogicalDecodingContext *ctx, ReorderBufferTXN *txn, { appendStringInfoString(ctx->out, " old-key:"); tuple_to_stringinfo(ctx->out, tupdesc, - &change->data.tp.oldtuple->tuple, + change->data.tp.oldtuple, true); appendStringInfoString(ctx->out, " new-tuple:"); } @@ -658,7 +658,7 @@ pg_decode_change(LogicalDecodingContext *ctx, ReorderBufferTXN *txn, appendStringInfoString(ctx->out, " (no-tuple-data)"); else tuple_to_stringinfo(ctx->out, tupdesc, - &change->data.tp.newtuple->tuple, + change->data.tp.newtuple, false); break; case REORDER_BUFFER_CHANGE_DELETE: @@ -670,7 +670,7 @@ pg_decode_change(LogicalDecodingContext *ctx, ReorderBufferTXN *txn, /* In DELETE, only the replica identity is present; display that */ else tuple_to_stringinfo(ctx->out, tupdesc, - &change->data.tp.oldtuple->tuple, + change->data.tp.oldtuple, true); break; default: diff --git a/src/backend/replication/logical/decode.c b/src/backend/replication/logical/decode.c index 6a0f22b209..760bc86a0b 100644 --- a/src/backend/replication/logical/decode.c +++ b/src/backend/replication/logical/decode.c @@ -62,7 +62,7 @@ static void DecodePrepare(LogicalDecodingContext *ctx, XLogRecordBuffer *buf, /* common function to decode tuples */ -static void DecodeXLogTuple(char *data, Size len, ReorderBufferTupleBuf *tuple); +static void DecodeXLogTuple(char *data, Size len, HeapTuple tuple); /* helper functions for decoding transactions */ static inline bool FilterPrepare(LogicalDecodingContext *ctx, @@ -155,7 +155,7 @@ xlog_decode(LogicalDecodingContext *ctx, XLogRecordBuffer *buf) case XLOG_PARAMETER_CHANGE: { xl_parameter_change *xlrec = - (xl_parameter_change *) XLogRecGetData(buf->record); + (xl_parameter_change *) XLogRecGetData(buf->record); /* * If wal_level on the primary is reduced to less than @@ -1152,7 +1152,7 @@ DecodeMultiInsert(LogicalDecodingContext *ctx, XLogRecordBuffer *buf) ReorderBufferChange *change; xl_multi_insert_tuple *xlhdr; int datalen; - ReorderBufferTupleBuf *tuple; + HeapTuple tuple; HeapTupleHeader header; change = ReorderBufferGetChange(ctx->reorder); @@ -1169,21 +1169,21 @@ DecodeMultiInsert(LogicalDecodingContext *ctx, XLogRecordBuffer *buf) ReorderBufferGetTupleBuf(ctx->reorder, datalen); tuple = change->data.tp.newtuple; - header = tuple->tuple.t_data; + header = tuple->t_data; /* not a disk based tuple */ - ItemPointerSetInvalid(&tuple->tuple.t_self); + ItemPointerSetInvalid(&tuple->t_self); /* * We can only figure this out after reassembling the transactions. */ - tuple->tuple.t_tableOid = InvalidOid; + tuple->t_tableOid = InvalidOid; - tuple->tuple.t_len = datalen + SizeofHeapTupleHeader; + tuple->t_len = datalen + SizeofHeapTupleHeader; memset(header, 0, SizeofHeapTupleHeader); - memcpy((char *) tuple->tuple.t_data + SizeofHeapTupleHeader, + memcpy((char *) tuple->t_data + SizeofHeapTupleHeader, (char *) data, datalen); header->t_infomask = xlhdr->t_infomask; @@ -1253,7 +1253,7 @@ DecodeSpecConfirm(LogicalDecodingContext *ctx, XLogRecordBuffer *buf) * computed outside as they are record specific. */ static void -DecodeXLogTuple(char *data, Size len, ReorderBufferTupleBuf *tuple) +DecodeXLogTuple(char *data, Size len, HeapTuple tuple) { xl_heap_header xlhdr; int datalen = len - SizeOfHeapHeader; @@ -1261,14 +1261,14 @@ DecodeXLogTuple(char *data, Size len, ReorderBufferTupleBuf *tuple) Assert(datalen >= 0); - tuple->tuple.t_len = datalen + SizeofHeapTupleHeader; - header = tuple->tuple.t_data; + tuple->t_len = datalen + SizeofHeapTupleHeader; + header = tuple->t_data; /* not a disk based tuple */ - ItemPointerSetInvalid(&tuple->tuple.t_self); + ItemPointerSetInvalid(&tuple->t_self); /* we can only figure this out after reassembling the transactions */ - tuple->tuple.t_tableOid = InvalidOid; + tuple->t_tableOid = InvalidOid; /* data is not stored aligned, copy to aligned storage */ memcpy((char *) &xlhdr, @@ -1277,7 +1277,7 @@ DecodeXLogTuple(char *data, Size len, ReorderBufferTupleBuf *tuple) memset(header, 0, SizeofHeapTupleHeader); - memcpy(((char *) tuple->tuple.t_data) + SizeofHeapTupleHeader, + memcpy(((char *) tuple->t_data) + SizeofHeapTupleHeader, data + SizeOfHeapHeader, datalen); diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c index d1334ffb55..d62a0758e9 100644 --- a/src/backend/replication/logical/reorderbuffer.c +++ b/src/backend/replication/logical/reorderbuffer.c @@ -498,13 +498,13 @@ ReorderBufferReturnChange(ReorderBuffer *rb, ReorderBufferChange *change, case REORDER_BUFFER_CHANGE_INTERNAL_SPEC_INSERT: if (change->data.tp.newtuple) { - ReorderBufferReturnTupleBuf(rb, change->data.tp.newtuple); + ReorderBufferReturnTupleBuf(change->data.tp.newtuple); change->data.tp.newtuple = NULL; } if (change->data.tp.oldtuple) { - ReorderBufferReturnTupleBuf(rb, change->data.tp.oldtuple); + ReorderBufferReturnTupleBuf(change->data.tp.oldtuple); change->data.tp.oldtuple = NULL; } break; @@ -547,36 +547,26 @@ ReorderBufferReturnChange(ReorderBuffer *rb, ReorderBufferChange *change, } /* - * Get a fresh ReorderBufferTupleBuf fitting at least a tuple of size + * Get a fresh HeapTuple fitting at least a tuple of size * tuple_len (excluding header overhead). */ -ReorderBufferTupleBuf * +HeapTuple ReorderBufferGetTupleBuf(ReorderBuffer *rb, Size tuple_len) { - ReorderBufferTupleBuf *tuple; + HeapTuple tuple; Size alloc_len; - alloc_len = tuple_len + SizeofHeapTupleHeader; + alloc_len = tuple_len + HEAPTUPLESIZE; - tuple = (ReorderBufferTupleBuf *) + tuple = (HeapTuple) MemoryContextAlloc(rb->tup_context, - sizeof(ReorderBufferTupleBuf) + + HEAPTUPLESIZE + MAXIMUM_ALIGNOF + alloc_len); - tuple->alloc_tuple_size = alloc_len; - tuple->tuple.t_data = ReorderBufferTupleBufData(tuple); + tuple->t_data = (HeapTupleHeader)((char *)tuple + HEAPTUPLESIZE); return tuple; } -/* - * Free a ReorderBufferTupleBuf. - */ -void -ReorderBufferReturnTupleBuf(ReorderBuffer *rb, ReorderBufferTupleBuf *tuple) -{ - pfree(tuple); -} - /* * Get an array for relids of truncated relations. * @@ -3759,8 +3749,8 @@ ReorderBufferSerializeChange(ReorderBuffer *rb, ReorderBufferTXN *txn, case REORDER_BUFFER_CHANGE_INTERNAL_SPEC_INSERT: { char *data; - ReorderBufferTupleBuf *oldtup, - *newtup; + HeapTuple oldtup, + newtup; Size oldlen = 0; Size newlen = 0; @@ -3770,14 +3760,14 @@ ReorderBufferSerializeChange(ReorderBuffer *rb, ReorderBufferTXN *txn, if (oldtup) { sz += sizeof(HeapTupleData); - oldlen = oldtup->tuple.t_len; + oldlen = oldtup->t_len; sz += oldlen; } if (newtup) { sz += sizeof(HeapTupleData); - newlen = newtup->tuple.t_len; + newlen = newtup->t_len; sz += newlen; } @@ -3790,19 +3780,19 @@ ReorderBufferSerializeChange(ReorderBuffer *rb, ReorderBufferTXN *txn, if (oldlen) { - memcpy(data, &oldtup->tuple, sizeof(HeapTupleData)); + memcpy(data, oldtup, sizeof(HeapTupleData)); data += sizeof(HeapTupleData); - memcpy(data, oldtup->tuple.t_data, oldlen); + memcpy(data, oldtup->t_data, oldlen); data += oldlen; } if (newlen) { - memcpy(data, &newtup->tuple, sizeof(HeapTupleData)); + memcpy(data, newtup, sizeof(HeapTupleData)); data += sizeof(HeapTupleData); - memcpy(data, newtup->tuple.t_data, newlen); + memcpy(data, newtup->t_data, newlen); data += newlen; } break; @@ -4118,8 +4108,8 @@ ReorderBufferChangeSize(ReorderBufferChange *change) case REORDER_BUFFER_CHANGE_DELETE: case REORDER_BUFFER_CHANGE_INTERNAL_SPEC_INSERT: { - ReorderBufferTupleBuf *oldtup, - *newtup; + HeapTuple oldtup, + newtup; Size oldlen = 0; Size newlen = 0; @@ -4129,14 +4119,14 @@ ReorderBufferChangeSize(ReorderBufferChange *change) if (oldtup) { sz += sizeof(HeapTupleData); - oldlen = oldtup->tuple.t_len; + oldlen = oldtup->t_len; sz += oldlen; } if (newtup) { sz += sizeof(HeapTupleData); - newlen = newtup->tuple.t_len; + newlen = newtup->t_len; sz += newlen; } @@ -4365,16 +4355,16 @@ ReorderBufferRestoreChange(ReorderBuffer *rb, ReorderBufferTXN *txn, ReorderBufferGetTupleBuf(rb, tuplelen - SizeofHeapTupleHeader); /* restore ->tuple */ - memcpy(&change->data.tp.oldtuple->tuple, data, + memcpy(change->data.tp.oldtuple, data, sizeof(HeapTupleData)); data += sizeof(HeapTupleData); /* reset t_data pointer into the new tuplebuf */ - change->data.tp.oldtuple->tuple.t_data = - ReorderBufferTupleBufData(change->data.tp.oldtuple); + change->data.tp.oldtuple->t_data = + (HeapTupleHeader)((char*)change->data.tp.oldtuple + HEAPTUPLESIZE); /* restore tuple data itself */ - memcpy(change->data.tp.oldtuple->tuple.t_data, data, tuplelen); + memcpy(change->data.tp.oldtuple->t_data, data, tuplelen); data += tuplelen; } @@ -4390,16 +4380,16 @@ ReorderBufferRestoreChange(ReorderBuffer *rb, ReorderBufferTXN *txn, ReorderBufferGetTupleBuf(rb, tuplelen - SizeofHeapTupleHeader); /* restore ->tuple */ - memcpy(&change->data.tp.newtuple->tuple, data, + memcpy(change->data.tp.newtuple, data, sizeof(HeapTupleData)); data += sizeof(HeapTupleData); /* reset t_data pointer into the new tuplebuf */ - change->data.tp.newtuple->tuple.t_data = - ReorderBufferTupleBufData(change->data.tp.newtuple); + change->data.tp.newtuple->t_data = + (HeapTupleHeader)((char*)change->data.tp.newtuple + HEAPTUPLESIZE); /* restore tuple data itself */ - memcpy(change->data.tp.newtuple->tuple.t_data, data, tuplelen); + memcpy(change->data.tp.newtuple->t_data, data, tuplelen); data += tuplelen; } @@ -4646,7 +4636,7 @@ ReorderBufferToastAppendChunk(ReorderBuffer *rb, ReorderBufferTXN *txn, Relation relation, ReorderBufferChange *change) { ReorderBufferToastEnt *ent; - ReorderBufferTupleBuf *newtup; + HeapTuple newtup; bool found; int32 chunksize; bool isnull; @@ -4661,9 +4651,9 @@ ReorderBufferToastAppendChunk(ReorderBuffer *rb, ReorderBufferTXN *txn, Assert(IsToastRelation(relation)); newtup = change->data.tp.newtuple; - chunk_id = DatumGetObjectId(fastgetattr(&newtup->tuple, 1, desc, &isnull)); + chunk_id = DatumGetObjectId(fastgetattr(newtup, 1, desc, &isnull)); Assert(!isnull); - chunk_seq = DatumGetInt32(fastgetattr(&newtup->tuple, 2, desc, &isnull)); + chunk_seq = DatumGetInt32(fastgetattr(newtup, 2, desc, &isnull)); Assert(!isnull); ent = (ReorderBufferToastEnt *) @@ -4686,7 +4676,7 @@ ReorderBufferToastAppendChunk(ReorderBuffer *rb, ReorderBufferTXN *txn, elog(ERROR, "got sequence entry %d for toast chunk %u instead of seq %d", chunk_seq, chunk_id, ent->last_chunk_seq + 1); - chunk = DatumGetPointer(fastgetattr(&newtup->tuple, 3, desc, &isnull)); + chunk = DatumGetPointer(fastgetattr(newtup, 3, desc, &isnull)); Assert(!isnull); /* calculate size so we can allocate the right size at once later */ @@ -4737,7 +4727,7 @@ ReorderBufferToastReplace(ReorderBuffer *rb, ReorderBufferTXN *txn, Relation toast_rel; TupleDesc toast_desc; MemoryContext oldcontext; - ReorderBufferTupleBuf *newtup; + HeapTuple newtup; Size old_size; /* no toast tuples changed */ @@ -4777,7 +4767,7 @@ ReorderBufferToastReplace(ReorderBuffer *rb, ReorderBufferTXN *txn, newtup = change->data.tp.newtuple; - heap_deform_tuple(&newtup->tuple, desc, attrs, isnull); + heap_deform_tuple(newtup, desc, attrs, isnull); for (natt = 0; natt < desc->natts; natt++) { @@ -4842,12 +4832,12 @@ ReorderBufferToastReplace(ReorderBuffer *rb, ReorderBufferTXN *txn, { bool cisnull; ReorderBufferChange *cchange; - ReorderBufferTupleBuf *ctup; + HeapTuple ctup; Pointer chunk; cchange = dlist_container(ReorderBufferChange, node, it.cur); ctup = cchange->data.tp.newtuple; - chunk = DatumGetPointer(fastgetattr(&ctup->tuple, 3, toast_desc, &cisnull)); + chunk = DatumGetPointer(fastgetattr(ctup, 3, toast_desc, &cisnull)); Assert(!cisnull); Assert(!VARATT_IS_EXTERNAL(chunk)); @@ -4882,11 +4872,11 @@ ReorderBufferToastReplace(ReorderBuffer *rb, ReorderBufferTXN *txn, * the tuplebuf because attrs[] will point back into the current content. */ tmphtup = heap_form_tuple(desc, attrs, isnull); - Assert(newtup->tuple.t_len <= MaxHeapTupleSize); - Assert(ReorderBufferTupleBufData(newtup) == newtup->tuple.t_data); + Assert(newtup->t_len <= MaxHeapTupleSize); + Assert(newtup->t_data == (HeapTupleHeader)((char*)newtup + HEAPTUPLESIZE)); - memcpy(newtup->tuple.t_data, tmphtup->t_data, tmphtup->t_len); - newtup->tuple.t_len = tmphtup->t_len; + memcpy(newtup->t_data, tmphtup->t_data, tmphtup->t_len); + newtup->t_len = tmphtup->t_len; /* * free resources we won't further need, more persistent stuff will be diff --git a/src/backend/replication/pgoutput/pgoutput.c b/src/backend/replication/pgoutput/pgoutput.c index 425238187f..998f92d671 100644 --- a/src/backend/replication/pgoutput/pgoutput.c +++ b/src/backend/replication/pgoutput/pgoutput.c @@ -1473,7 +1473,7 @@ pgoutput_change(LogicalDecodingContext *ctx, ReorderBufferTXN *txn, if (change->data.tp.oldtuple) { old_slot = relentry->old_slot; - ExecStoreHeapTuple(&change->data.tp.oldtuple->tuple, old_slot, false); + ExecStoreHeapTuple(change->data.tp.oldtuple, old_slot, false); /* Convert tuple if needed. */ if (relentry->attrmap) @@ -1488,7 +1488,7 @@ pgoutput_change(LogicalDecodingContext *ctx, ReorderBufferTXN *txn, if (change->data.tp.newtuple) { new_slot = relentry->new_slot; - ExecStoreHeapTuple(&change->data.tp.newtuple->tuple, new_slot, false); + ExecStoreHeapTuple(change->data.tp.newtuple, new_slot, false); /* Convert tuple if needed. */ if (relentry->attrmap) diff --git a/src/include/replication/reorderbuffer.h b/src/include/replication/reorderbuffer.h index 3e232c6c27..efa57f2bec 100644 --- a/src/include/replication/reorderbuffer.h +++ b/src/include/replication/reorderbuffer.h @@ -28,25 +28,6 @@ typedef enum DEBUG_LOGICAL_REP_STREAMING_IMMEDIATE, } DebugLogicalRepStreamingMode; -/* an individual tuple, stored in one chunk of memory */ -typedef struct ReorderBufferTupleBuf -{ - /* position in preallocated list */ - slist_node node; - - /* tuple header, the interesting bit for users of logical decoding */ - HeapTupleData tuple; - - /* pre-allocated size of tuple buffer, different from tuple size */ - Size alloc_tuple_size; - - /* actual tuple data follows */ -} ReorderBufferTupleBuf; - -/* pointer to the data stored in a TupleBuf */ -#define ReorderBufferTupleBufData(p) \ - ((HeapTupleHeader) MAXALIGN(((char *) p) + sizeof(ReorderBufferTupleBuf))) - /* * Types of the change passed to a 'change' callback. * @@ -114,9 +95,9 @@ typedef struct ReorderBufferChange bool clear_toast_afterwards; /* valid for DELETE || UPDATE */ - ReorderBufferTupleBuf *oldtuple; + HeapTuple oldtuple; /* valid for INSERT || UPDATE */ - ReorderBufferTupleBuf *newtuple; + HeapTuple newtuple; } tp; /* @@ -678,10 +659,18 @@ struct ReorderBuffer extern ReorderBuffer *ReorderBufferAllocate(void); extern void ReorderBufferFree(ReorderBuffer *rb); -extern ReorderBufferTupleBuf *ReorderBufferGetTupleBuf(ReorderBuffer *rb, - Size tuple_len); -extern void ReorderBufferReturnTupleBuf(ReorderBuffer *rb, - ReorderBufferTupleBuf *tuple); +extern HeapTuple ReorderBufferGetTupleBuf(ReorderBuffer *rb, + Size tuple_len); + +/* + * Free HeapTuple returned by ReorderBufferGetTupleBuf(). + */ +static inline void +ReorderBufferReturnTupleBuf(HeapTuple tuple) +{ + pfree(tuple); +} + extern ReorderBufferChange *ReorderBufferGetChange(ReorderBuffer *rb); extern void ReorderBufferReturnChange(ReorderBuffer *rb, ReorderBufferChange *change, bool upd_mem); diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index 7e866e3c3d..dc3b0ef871 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -2351,7 +2351,6 @@ ReorderBufferStreamTruncateCB ReorderBufferTXN ReorderBufferTXNByIdEnt ReorderBufferToastEnt -ReorderBufferTupleBuf ReorderBufferTupleCidEnt ReorderBufferTupleCidKey ReorderBufferUpdateProgressTxnCB -- 2.43.0