On Mon, Sep 9, 2019 at 4:02 PM Paul Guo <p...@pivotal.io> wrote: > > So in theory > we should not worry about additional tuple copy overhead now, and then I tried the patch without setting > multi-insert threshold as attached. >
I reviewed your patch today. It looks good overall. My concern is that the ExecFetchSlotHeapTuple call does not seem appropriate. In a generic place such as createas.c, we should be using generic tableam API only. However, I can also see that there is no better alternative. We need to compute the size of accumulated tuples so far, in order to decide whether to stop accumulating tuples. There is no convenient way to obtain the length of the tuple, given a slot. How about making that decision solely based on number of tuples, so that we can avoid ExecFetchSlotHeapTuple call altogether? The multi insert copy code deals with index tuples also, which I don't see in the patch. Don't we need to consider populating indexes? Asim
