On Wed, Nov 13, 2019 at 05:55:12PM -0800, Andres Freund wrote: > My point was that I think there must be negation missing in Daniel's > statement - XLH_INSERT_CONTAINS_NEW_TUPLE will only be set if *not* a > catalog relation. But Daniel's statement says exactly the opposite, at > least by my read.
FWIW, I am reading the same, aka the sentence of Daniel is wrong. And
that what you say is right.
> I can't follow what you're trying to get at in this sub discussion - why
> would we want to sanity check something about catalog tables here? Given
> that XLH_INSERT_CONTAINS_NEW_TUPLE is set exactly when the table is not
> a catalog table, that seems entirely superfluous?
[ ... Looking ... ]
You are right, we could just rely on cross-checking that when we have
no data then XLH_INSERT_CONTAINS_NEW_TUPLE is not set, or something
like that:
@@ -901,11 +901,17 @@ 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.
+ * multi_insert can be used by catalogs, hence it is possible that
+ * the block does not have any data even if a full-page write of it
+ * is taken.
*/
tupledata = XLogRecGetBlockData(r, 0, &tuplelen);
- Assert(tupledata != NULL);
+ Assert(tupledata == NULL ||
+ (xlrec->flags & XLH_INSERT_CONTAINS_NEW_TUPLE) != 0);
+
+ /* No data, then leave */
+ if (tupledata == NULL)
+ return;
The latest patch does not apply, so it needs a rebase.
--
Michael
signature.asc
Description: PGP signature
