On Thu, Nov 26, 2020 at 07:24:01AM +0530, Bharath Rupireddy wrote:
> Yeah. The tuple size may change after ExecCopySlot(). For instance, create
> table t2 as select a1 from t1; where t1 has two integer columns a1, b1. I'm
> creating t2 with single column a1 from t1 which makes the source slot
> virtual.

+inline Size
+GetTupleSize(TupleTableSlot *slot, Size maxsize)
+{
+   Size sz = 0;
+   HeapTuple tuple = NULL;
+
+   if (TTS_IS_HEAPTUPLE(slot))
+       tuple = ((HeapTupleTableSlot *) slot)->tuple;
+   else if(TTS_IS_BUFFERTUPLE(slot))
+       tuple = ((BufferHeapTupleTableSlot *) slot)->base.tuple;
+   else if(TTS_IS_MINIMALTUPLE(slot))
+       tuple = ((MinimalTupleTableSlot *) slot)->tuple;

There have been various talks about the methods we could use to
evaluate the threshold in bytes when evaluating that a flush can
happen, including the use of memory contexts, or even estimate the
size of the number of tuples.  This one looks promising because it
seems exact, however for virtual slots I don't like much the fact that
you basically just extracted the parts of tts_virtual_materialize()
and stuck them in this routine.  That's a recipe for future bugs if
the materialization logic changes.  In short, I am surprised that this
calculation is not directly part of TupleTableSlotOps.  What we'd want
is to get this information depending on the slot type dealt with, and
with your patch you would miss to handle any new slot type
introduced.
--
Michael

Attachment: signature.asc
Description: PGP signature

Reply via email to