Skip site navigation (1) Skip section navigation (2)

Re: review: FDW API

From: Shigeru HANADA <hanada(at)metrosystems(dot)co(dot)jp>
To: Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>
Cc: Jan Urbański <wulczer(at)wulczer(dot)org>,Postgres - Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: review: FDW API
Date: 2011-01-31 03:27:55
Message-ID: 20110131122754.9B20.6989961C@metrosystems.co.jp (view raw or flat)
Thread:
Lists: pgsql-hackers
On Fri, 21 Jan 2011 18:28:19 +0200
Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com> wrote:
> On 18.01.2011 17:26, Shigeru HANADA wrote:
> > 3) 20110118-fdw_handler.patch - This patch adds support for HANDLER
> > option to FOREIGN DATA WRAPPER object.
> 
> Some quick comments on that:

Thanks for the comments.
I'll post revised version of patches soon.

> * The elogs in parse_func_options() should be ereports.
> 
> * pg_dump should check the version number and only try to select 
> fdwhandler column if >= 9.1. See the other functions there for example 
> of that.

Fixed.

> * dumpForeignDataWrapper() in pg_dump checks if fdwhandler field is "-". 
> I don't think we use that as magic value there, do we? Same with validator.

That magic value, "-",  is used as "no-function-was-set" in
dumpForeignDataWrapper() and dumpForeignServer(), and I followed them.
Agreed that magic value should be removed, but it would be a refactoring
issue about pg_dump.

> * Please check that the HANDLER and VALIDATOR options that pg_dump 
> creates properly quoted.

I checked quoting for HANDLER and VALIDATOR with file_fdw functions,
and it seems works fine.  The pg_dump generats:

    ------------
    CREATE FOREIGN DATA WRAPPER dummy_fdw VALIDATOR public."File_Fdw_Validator"
    HANDLER public."FILE_FDW_HANDLER";
    ------------

    from these DDLs:

    ------------
    CREATE OR REPLACE FUNCTION "File_Fdw_Validator" (text[], oid)
    RETURNS bool
    AS '$libdir/file_fdw','file_fdw_validator'
    LANGUAGE C STRICT;

    CREATE OR REPLACE FUNCTION "FILE_FDW_HANDLER" ()
    RETURNS fdw_handler
    AS '$libdir/file_fdw','file_fdw_handler'
    LANGUAGE C STRICT;

    CREATE FOREIGN DATA WRAPPER dummy_fdw
    VALIDATOR "File_Fdw_Validator" HANDLER "FILE_FDW_HANDLER";
    ------------

Regard,
--
Shigeru Hanada



In response to

pgsql-hackers by date

Next:From: Itagaki TakahiroDate: 2011-01-31 03:31:12
Subject: Re: pg_ctl failover Re: Latches, signals, and waiting
Previous:From: Robert HaasDate: 2011-01-31 03:26:01
Subject: Re: We need to log aborted autovacuums

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group