On 2019-Sep-25, Asim R P wrote: > 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?
... maybe we should add a new operation to slots, that returns the (approximate?) size of a tuple? That would make this easy. (I'm not sure however what to do about TOAST considerations -- is it size in memory that we're worried about?) Also: + myState->mi_slots_size >= 65535) This magic number should be placed in a define next to the other one, but I'm not sure that heapam.h is a good location, since surely this applies to matviews in other table AMs too. -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
