Re: SQL/MED - file_fdw - Mailing list pgsql-hackers

From Itagaki Takahiro
Subject Re: SQL/MED - file_fdw
Date
Msg-id AANLkTinK_YfNFQzcx_v8mhycwU-cGYThg2f9zzCgV5t8@mail.gmail.com
Whole thread Raw
In response to Re: SQL/MED - file_fdw  (Itagaki Takahiro <itagaki.takahiro@gmail.com>)
Responses Re: SQL/MED - file_fdw  (Shigeru HANADA <hanada@metrosystems.co.jp>)
List pgsql-hackers
On Tue, Dec 21, 2010 at 21:32, Itagaki Takahiro
<itagaki.takahiro@gmail.com> wrote:
> On Tue, Dec 21, 2010 at 20:14, Shigeru HANADA <hanada@metrosystems.co.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


pgsql-hackers by date:

Previous
From: Josh Berkus
Date:
Subject: Re: Streaming replication as a separate permissions
Next
From: Florian Pflug
Date:
Subject: Re: Streaming replication as a separate permissions