Re: SQL/MED - file_fdw

On Tue, Dec 21, 2010 at 21:32, Itagaki Takahiro
<itagaki(dot)takahiro(at)gmail(dot)com> wrote:
> On Tue, Dec 21, 2010 at 20:14, Shigeru HANADA <hanada(at)metrosystems(dot)co(dot)jp> wrote:
>> Attached is the revised version of file_fdw patch.  This patch is
>> based on Itagaki-san's copy_export-20101220.diff patch.
>
> #1. Don't you have per-tuple memory leak? I added GetCopyExecutorState()
> because the caller needs to reset the per-tuple context periodically.

Sorry, I found there are no memory leak here. The related comment is:
[execnodes.h]
* CurrentMemoryContext should be set to ecxt_per_tuple_memory before
* calling ExecEvalExpr() --- see ExecEvalExprSwitchContext().
I guess CurrentMemoryContext in Iterate callback a per-tuple context.
So, we don't have to xport cstate->estate via GetCopyExecutorState().

> Or, if you eventually make a HeapTuple from values and nulls arrays,

ExecStoreVirtualTuple() seems to be better than the combination of
heap_form_tuple() and ExecStoreTuple() for the purpose. Could you try
to use slot->tts_values and slot->tts_isnull for NextCopyFrom() directly?

--
Itagaki Takahiro