Re: Assertion for logically decoding multi inserts into the catalog

On 08/07/2019 22:42, Daniel Gustafsson wrote:
> My patch for using heap_multi_insert in the catalog [1] failed the logical
> decoding part of test/recovery [2].
>
> The assertion it failed on seems to not take multi inserts into the catalog
> into consideration, while the main logic does. This assertion hasn't tripped
> since there are no multi inserts into the catalog, but if we introduce them it
> will so I’m raising it in a separate thread as it is sort of unrelated from the
> patch in question.
>
> The attached patch fixes my test failure and makes sense to me, but this code
> is far from my neck of the tree, so I’m really not sure this is the best way to
> express the assertion.
>
> --- a/src/backend/replication/logical/decode.c
> +++ b/src/backend/replication/logical/decode.c
> @@ -974,7 +974,8 @@ DecodeMultiInsert(LogicalDecodingContext *ctx, XLogRecordBuffer *buf)
> ReorderBufferQueueChange(ctx->reorder, XLogRecGetXid(r),
> buf->origptr, change);
> }
> - Assert(data == tupledata + tuplelen);
> + Assert(xlrec->flags & XLH_INSERT_CONTAINS_NEW_TUPLE &&
> + data == tupledata + tuplelen);
> }
>
> /*

This patch makes the assertion more strict than it was before. I don't
see how it could possibly make a regression failure go away. On the
contrary. So, huh?

- Heikki