Re: FDW for PostgreSQL

Thank for the comment!

On Tue, Nov 20, 2012 at 10:23 PM, Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp> wrote:

> I also think the new "use_remote_explain" option is good. It works fine
> when we try to use this fdw over the network with latency more or less.
> It seems to me its default is "false", thus, GetForeignRelSize will return
> the estimated cost according to ANALYZE, instead of remote EXPLAIN.
> Even though you mention the default behavior was not changed, is it
> an expected one? My preference is the current one, as is.
>

Oops, I must have focused on only cost factors.
I too think that using local statistics is better as default behavior,
because foreign tables would be used for relatively stable tables.
If target tables are updated often, it would cause problems about
consistency, unless we provide full-fledged transaction mapping.

> The deparseFuncExpr() still has case handling whether it is explicit case,
> implicit cast or regular functions. If my previous proposition has no flaw,
> could you fix up it using regular function invocation manner? In case when
> remote node has incompatible implicit-cast definition, this logic can make
> a problem.
>

Sorry, I overlooked this issue. Fixed to use function call notation
for all of explicit function calls, explicit casts, and implicit casts.

At InitPostgresFdwOptions(), the source comment says we don't use
> malloc() here for simplification of code. Hmm. I'm not sure why it is more
> simple. It seems to me we have no reason why to avoid malloc here, even
> though libpq options are acquired using malloc().
>

I used "simple" because using palloc avoids null-check and error handling.
However, many backend code use malloc to allocate memory which lives
as long as backend process itself, so I fixed.

> Regarding to the regression test.
>
[snip]

> I guess this test tries to check a case when remote column has incompatible
> data type with local side. Please check timestamp_out(). Its output
> format follows
> "datestyle" setting of GUC, and it affects OS configuration on initdb time.
> (Please grep "datestyle" at initdb.c !) I'd like to recommend to use
> another data type
> for this regression test to avoid false-positive detection.
>

Good catch. :)
I fixed the test to use another data type, user defined enum.

Regards,
--
Shigeru HANADA