Re: Batch insert in CTAS/MatView code

From: Paul Guo <pguo(at)pivotal(dot)io>
To: Michael Paquier <michael(at)paquier(dot)xyz>
Cc: Andres Freund <andres(at)anarazel(dot)de>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Asim R P <apraveen(at)pivotal(dot)io>, Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org>, Taylor Vesely <tvesely(at)pivotal(dot)io>
Subject: Re: Batch insert in CTAS/MatView code
Date: 2020-01-17 07:02:06
Message-ID: CAEET0ZGw=0fr0pm4hDkLp+ptYBccBFyaxncRWBMa9+0En-UdmA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: 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().

+ ExecCopySlot(batchslot, slot);
+ 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;
+}

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.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Fujii Masao 2020-01-17 07:05:03 Re: Add pg_file_sync() to adminpack
Previous Message Pavel Stehule 2020-01-17 06:54:29 Re: SQL/JSON: functions