From e3e7a720d7325a477e6d189d34619f05bdf568df Mon Sep 17 00:00:00 2001 From: Daniel Gustafsson Date: Tue, 25 Feb 2020 22:21:40 +0100 Subject: [PATCH 2/2] Fix DecodeMultiInsert to handle catalog multi inserts The coding in DecodeMultiInsert assumed that multi inserts weren't used for catalogs, and would fail on tupledata being NULL in case they were. This is currently safe as there are no multi inserts for the catalog, but that might change leading to this codepath falling over. --- src/backend/replication/logical/decode.c | 63 +++++++++++------------- 1 file changed, 30 insertions(+), 33 deletions(-) diff --git a/src/backend/replication/logical/decode.c b/src/backend/replication/logical/decode.c index 5e1dc8a651..140d9a0710 100644 --- a/src/backend/replication/logical/decode.c +++ b/src/backend/replication/logical/decode.c @@ -891,6 +891,14 @@ DecodeMultiInsert(LogicalDecodingContext *ctx, XLogRecordBuffer *buf) xlrec = (xl_heap_multi_insert *) XLogRecGetData(r); + /* + * CONTAINS_NEW_TUPLE will always be set unless the multi_insert was + * performed for a catalog. If it is a catalog, return immediately as + * there is nothing to logically decode. + */ + if (!(xlrec->flags & XLH_INSERT_CONTAINS_NEW_TUPLE)) + return; + /* only interested in our database */ XLogRecGetBlockTag(r, 0, &rnode, NULL, NULL); if (rnode.dbNode != ctx->slot->data.database) @@ -901,8 +909,8 @@ DecodeMultiInsert(LogicalDecodingContext *ctx, XLogRecordBuffer *buf) return; /* - * As multi_insert is not used for catalogs yet, the block should always - * have data even if a full-page write of it is taken. + * We know that this multi_insert isn't for a catalog, so the block should + * always have data even if a full-page write of it is taken. */ tupledata = XLogRecGetBlockData(r, 0, &tuplelen); Assert(tupledata != NULL); @@ -914,6 +922,7 @@ DecodeMultiInsert(LogicalDecodingContext *ctx, XLogRecordBuffer *buf) xl_multi_insert_tuple *xlhdr; int datalen; ReorderBufferTupleBuf *tuple; + HeapTupleHeader header; change = ReorderBufferGetChange(ctx->reorder); change->action = REORDER_BUFFER_CHANGE_INSERT; @@ -925,43 +934,31 @@ DecodeMultiInsert(LogicalDecodingContext *ctx, XLogRecordBuffer *buf) data = ((char *) xlhdr) + SizeOfMultiInsertTuple; datalen = xlhdr->datalen; - /* - * CONTAINS_NEW_TUPLE will always be set currently as multi_insert - * isn't used for catalogs, but better be future proof. - * - * We decode the tuple in pretty much the same way as DecodeXLogTuple, - * but since the layout is slightly different, we can't use it here. - */ - if (xlrec->flags & XLH_INSERT_CONTAINS_NEW_TUPLE) - { - HeapTupleHeader header; - - change->data.tp.newtuple = - ReorderBufferGetTupleBuf(ctx->reorder, datalen); + change->data.tp.newtuple = + ReorderBufferGetTupleBuf(ctx->reorder, datalen); - tuple = change->data.tp.newtuple; - header = tuple->tuple.t_data; + tuple = change->data.tp.newtuple; + header = tuple->tuple.t_data; - /* not a disk based tuple */ - ItemPointerSetInvalid(&tuple->tuple.t_self); + /* not a disk based tuple */ + ItemPointerSetInvalid(&tuple->tuple.t_self); - /* - * We can only figure this out after reassembling the - * transactions. - */ - tuple->tuple.t_tableOid = InvalidOid; + /* + * We can only figure this out after reassembling the + * transactions. + */ + tuple->tuple.t_tableOid = InvalidOid; - tuple->tuple.t_len = datalen + SizeofHeapTupleHeader; + tuple->tuple.t_len = datalen + SizeofHeapTupleHeader; - memset(header, 0, SizeofHeapTupleHeader); + memset(header, 0, SizeofHeapTupleHeader); - memcpy((char *) tuple->tuple.t_data + SizeofHeapTupleHeader, - (char *) data, - datalen); - header->t_infomask = xlhdr->t_infomask; - header->t_infomask2 = xlhdr->t_infomask2; - header->t_hoff = xlhdr->t_hoff; - } + memcpy((char *) tuple->tuple.t_data + SizeofHeapTupleHeader, + (char *) data, + datalen); + header->t_infomask = xlhdr->t_infomask; + header->t_infomask2 = xlhdr->t_infomask2; + header->t_hoff = xlhdr->t_hoff; /* * Reset toast reassembly state only after the last row in the last -- 2.21.1 (Apple Git-122.3)