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
signature.asc
Description: PGP signature
