Em ter., 15 de nov. de 2022 às 04:02, Michael Paquier <michael@paquier.xyz> escreveu:
On Tue, Nov 15, 2022 at 09:57:26AM +0900, Michael Paquier wrote: > Anyway, multi-inserts are going to be solution better than > CatalogTupleInsertWithInfo() in some cases, because we would just > generate one WAL record of N inserts rather than N records with one > INSERT each.
Something that you did not consider in the initial patch is that we may finish by opening catalog indexes even in cases where this would not have happened on HEAD, as we may finish by doing nothing when copying the stats or updating them during an analyze, and that's not fine IMO. However it is easy enough to minimize the cost: just do a CatalogOpenIndexes() when absolutely required, and close things only if the indexes have been opened.
I find it very difficult not to have some tuple to be updated,
once inside CopyStatistics and the branch cost can get in the way,
but I don't object with your solution.
Then, there are the cases where it is worth switching to a multi-insert logic as these are going to manipulate more than 2 entries all the time: enum list addition and two code paths of tsearchcmds.c (where up to 16 entries can be lined up). This is a case-by-case analysis. For example, in the case of the enums, the number of elements is known in advance so it is possible to know the number of slots that would be used and initialize them. But that's not something you would do for the first tsearch bits where the data is built upon a scan so the slot init should be delayed. The second tsearch one can use a predictible approach, like the enums based on the number of known elements to insert.
Makes sense.
So I've given a try at all that, and finished with the attached. This patch finishes with a list of bullet points, so this had better be split into different commits, I guess. Thoughts?
Missed AddRoleMems?
Could you continue with CatalogTupleInsertWithInfo, what do you think?
