Re: Batch insert in CTAS/MatView code

Hi,

On 2019-09-09 18:31:54 +0800, Paul Guo wrote:
> diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
> index e9544822bf..8a844b3b5f 100644
> --- a/src/backend/access/heap/heapam.c
> +++ b/src/backend/access/heap/heapam.c
> @@ -2106,7 +2106,6 @@ heap_multi_insert(Relation relation, TupleTableSlot **slots, int ntuples,
>                                 CommandId cid, int options, BulkInsertState bistate)
>  {
>       TransactionId xid = GetCurrentTransactionId();
> -     HeapTuple  *heaptuples;
>       int                     i;
>       int                     ndone;
>       PGAlignedBlock scratch;
> @@ -2115,6 +2114,10 @@ heap_multi_insert(Relation relation, TupleTableSlot **slots, int ntuples,
>       Size            saveFreeSpace;
>       bool            need_tuple_data = RelationIsLogicallyLogged(relation);
>       bool            need_cids = RelationIsAccessibleInLogicalDecoding(relation);
> +     /* Declare it as static to let this memory be not on stack. */
> +     static HeapTuple        heaptuples[MAX_MULTI_INSERT_TUPLES];
> +
> +     Assert(ntuples <= MAX_MULTI_INSERT_TUPLES);
> 
>       /* currently not needed (thus unsupported) for heap_multi_insert() */
>       AssertArg(!(options & HEAP_INSERT_NO_LOGICAL));
> @@ -2124,7 +2127,6 @@ heap_multi_insert(Relation relation, TupleTableSlot **slots, int ntuples,
>                                                                                                  HEAP_DEFAULT_FILLFACTOR);
> 
>       /* Toast and set header data in all the slots */
> -     heaptuples = palloc(ntuples * sizeof(HeapTuple));
>       for (i = 0; i < ntuples; i++)
>       {
>               HeapTuple       tuple;

I don't think this is a good idea. We shouldn't unnecessarily allocate
8KB on the stack. Is there any actual evidence this is a performance
benefit? To me this just seems like it'll reduce the flexibility of the

API, without any benefit.  I'll also note that you've apparently not
updated tableam.h to document this new restriction.