Re: Ought to use heap_multi_insert() for pg_attribute/dependinsertions? - Mailing list pgsql-hackers

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