On Mon, Aug 10, 2020 at 05:32:21PM -0700, Andres Freund wrote:
> Do we really want to end up with several separate defines for different
> type of catalog batch inserts? That doesn't seem like a good
> thing. Think there should be a single define for all catalog bulk
> inserts.
Unlikely so, but I kept them separate to potentially lower the
threshold of 64kB for catalog rows that have a lower average size than
pg_attribute. catalog.h would be the natural location I would choose
for a single definition.
> Hm, would it be better to first iterate over the dependencies, compute
> the number of dependencies to be inserted, and then go ahead and create
> the right number of slots?
Not sure about that, but I am not wedded to the approach of the patch
either as the most consuming portion is the slot initialization/reset.
Computing the number of items in advance forces to go through the
dependency list twice, while doing a single pass makes the code
allocate 64 extra bytes for each slot not used. It is of course
better to avoid calling isObjectPinned() twice for each dependency, so
we could use a bitmap, or just simply build a secondary list of
dependencies that we are sure will be inserted after doing a first
pass to discard the unwanted entries.
> Seems several places have been modified to new APIs despite only
> covering a single dependency. Perhaps worth mentioning?
Yeah, I need to think more about this commit message.
--
Michael