Re: Ought to use heap_multi_insert() for pg_attribute/depend insertions?

On Tue, Nov 12, 2019 at 10:33:16AM -0800, Andres Freund wrote:
> On 2019-11-12 16:25:06 +0100, Daniel Gustafsson wrote:
>> On 11 Nov 2019, at 09:32, Michael Paquier <michael(at)paquier(dot)xyz> wrote:
>>
>>> This part has resulted in 75c1921, and we could just change
>>> DecodeMultiInsert() so as if there is no tuple data then we'd just
>>> leave. However, I don't feel completely comfortable with that either
>>> as it would be nice to still check that for normal relations we
>>> *always* have a FPW available.
>>
>> XLH_INSERT_CONTAINS_NEW_TUPLE will only be set in case of catalog relations
>> IIUC (that is, non logically decoded relations), so it seems to me that we can
>> have a fastpath out of DecodeMultiInsert() which inspects that flag without
>> problems. Is this proposal along the lines of what you were thinking?
>
> Maybe I'm missing something, but it's the opposite, no?

> bool need_tuple_data = RelationIsLogicallyLogged(relation);

Yep. This checks after IsCatalogRelation().

> ...
> if (need_tuple_data)
> xlrec->flags |= XLH_INSERT_CONTAINS_NEW_TUPLE;
>
> or am I misunderstanding what you mean?

Not sure that I can think about a good way to properly track if the
new tuple data is associated to a catalog or not, aka if the data is
expected all the time or not to get a good sanity check when doing the
multi-insert decoding. We could extend xl_multi_insert_tuple with a
flag to track that, but that seems like an overkill for a simple
sanity check..
--
Michael