Re: Batch insert in CTAS/MatView code - Mailing list pgsql-hackers
| From | Paul Guo |
|---|---|
| Subject | Re: Batch insert in CTAS/MatView code |
| Date | |
| Msg-id | CAEET0ZGw=0fr0pm4hDkLp+ptYBccBFyaxncRWBMa9+0En-UdmA@mail.gmail.com Whole thread Raw |
| In response to | Re: Batch insert in CTAS/MatView code (Michael Paquier <michael@paquier.xyz>) |
| Responses |
Re: Batch insert in CTAS/MatView code
(Daniel Gustafsson <daniel@yesql.se>)
|
| List | pgsql-hackers |
I took some time on digging the issue yesterday so the main concern of the
patch is to get the tuple length from ExecFetchSlotHeapTuple().
+ tuple = ExecFetchSlotHeapTuple(batchslot, true, NULL);
+
+ myState->mi_slots_num++;
+ myState->mi_slots_size += tuple->t_len;
We definitely should remove ExecFetchSlotHeapTuple() here but we need to
know the tuple length (at least rough length). One solution might be using
memory context stat info as mentioned, the code looks like this.
tup_len = MemoryContextUsedSize(batchslot->tts_mcxt);
ExecCopySlot(batchslot, slot);
ExecMaterializeSlot(batchslot);
tup_len = MemoryContextUsedSize(batchslot->tts_mcxt) - tup_len;
MemoryContextUsedSize() is added to calculate the total used size (simply hack to use the stats interface).
+int
+MemoryContextUsedSize(MemoryContext context)
+{
+ MemoryContextCounters total;
+
+ memset(&total, 0, sizeof(total));
+ context->methods->stats(context, NULL, NULL, &total);
+
+ return total.totalspace - total.freespace;
+}
+MemoryContextUsedSize(MemoryContext context)
+{
+ MemoryContextCounters total;
+
+ memset(&total, 0, sizeof(total));
+ context->methods->stats(context, NULL, NULL, &total);
+
+ return total.totalspace - total.freespace;
+}
This basically works but there are concerns:
The length is not accurate (though we do not need to be that accurate) since there are
some additional memory allocations, but we are not sure if the size is not much
larger than the real length for some slot types in the future and I'm not sure whether we
definitely allocate at least the tuple length in the memory context after materialization
for all slot types in the future. Last is that the code seems to be a bit ugly also.
As a reference, For "create table t1 as select * from t2", the above code returns
"tuple length" is 88 (real tuple length is 4).
Another solution is that maybe return the real size in ExecMaterializeSlot()? e.g.
ExecMaterializeSlot(slot, &tup_len); For this we probably want to store the length
in the slot struct for performance.
For the COPY case the tuple length is known in advance but I can image more cases
which do not know the size but need that for the multi_insert interface, at least I'm
wondering if we should use that in 'refresh matview' and fast path for Insert node
(I heard some complaints about the performance of "insert into tbl from select..."
from some of our users)? So the concern is not just for the case in this patch.
Besides, My colleagues Ashwin Agrawal and Adam Lee found maybe could
try raw_heap_insert() similar code for ctas and compare since it is do insert in
a new created table. That would involve more discussions, much more code
change and need to test more (stability and performance). So multi insert seems
to be a stable solution in a short time given that has been used in COPY for a
long time?
Whatever the solution for CTAS we need to address the concern of tuple size
for multi insert cases.
pgsql-hackers by date: