Re: ALTER TABLE ADD COLUMN fast default

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