Hi, Thanks for looking into this.
On Mon, Oct 28, 2024 at 8:18 PM Jingtang Zhang <mrdrivingd...@gmail.com> wrote: > > Just found that the initialization > seems redundant since we have used palloc0? > > > + istate = (HeapInsertState *) palloc0(sizeof(HeapInsertState)); > > + istate->bistate = NULL; > > + istate->mistate = NULL; Changed it to palloc() and explicit initializations of the members. With this, only TupleTableSlot's array in HeapMultiInsertState uses palloc0(), the rest all use explicit initializations. > Oh, another comments for v24-0001 patch: we are in heam AM now, should we use > something like HEAP_INSERT_BAS_BULKWRITE instead of using table AM option, > just like other heap AM options do? > > > + if ((state->options & TABLE_INSERT_BAS_BULKWRITE) != 0) > > + istate->bistate = GetBulkInsertState(); Defined HEAP_INSERT_BAS_BULKWRITE and used that in heapam.c similar to INSERT_SKIP_FSM, INSERT_FROZEN, NO_LOGICAL. > Little question about v24 0002 patch: would it be better to move the > implementation of TableModifyIsMultiInsertsSupported to somewhere for table AM > level? Seems it is a common function for future use, not a specific one for > matview. It's more tailored for CREATE TABLE AS and CREATE/REFRESH MATERIALIZED VIEW in the sense that no triggers, foreign table and partitioned table possible here. INSERT INTO SELECT and Logical Replication Apply will have a lot more conditions (e.g. RETURNING clause, triggers etc.) and they will need to be handled differently. So, I left TableModifyIsMultiInsertsSupported as-is in a common place in matview.c. Please find the attached v25 patch set. -- Bharath Rupireddy PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
v25-0001-Introduce-new-table-AM-for-multi-inserts.patch
Description: Binary data
v25-0002-Optimize-CTAS-CMV-RMV-with-new-multi-inserts-tab.patch
Description: Binary data
v25-0003-Use-new-multi-inserts-table-AM-for-COPY-.-FROM.patch
Description: Binary data
