On Mon, Mar 26, 2018 at 9:32 AM, Andrew Dunstan <andrew.dunstan@2ndquadrant.
com> wrote:
>
>
> Thanks for this, all looks good. Here is the consolidate patch
> rebased. If there are no further comments I propose to commit this in
> a few days time.
I have some comments with the committed patch.
@@ -663,7 +671,23 @@ ExecFetchSlotTuple(TupleTableSlot *slot)
* If we have a regular physical tuple then just return it.
*/
if (TTS_HAS_PHYSICAL_TUPLE(slot))
- return slot->tts_tuple;
+ {
+ if (HeapTupleHeaderGetNatts(slot->tts_tuple->t_data) <
+ slot->tts_tupleDescriptor->natts)
+ {
+ MemoryContext oldContext =
MemoryContextSwitchTo(slot->tts_mcxt);
+
+ slot->tts_tuple = heap_expand_tuple(slot->tts_tuple,
+
slot->tts_tupleDescriptor);
+ slot->tts_shouldFree = true;
+ MemoryContextSwitchTo(oldContext);
+ return slot->tts_tuple;
+ }
+ else
+ {
+ return slot->tts_tuple;
+ }
+ }
In the above scenario, directly replacing the slot->tts_tuple without
freeing the exisitng
tuple will unnecessarily increase the slot context memory size, this may
lead to a problem
if the same slot is used for many tuples. Better to use ExecStoreTuple()
function to update
the slot tuple.
Regards,
Hari Babu
Fujitsu Australia
