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