On Fri, 21 Jan 2011 18:28:19 +0200
Heikki Linnakangas <heikki.linnakangas@enterprisedb.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'
LANGUAGEC STRICT;
CREATE FOREIGN DATA WRAPPER dummy_fdw VALIDATOR "File_Fdw_Validator" HANDLER "FILE_FDW_HANDLER"; ------------
Regard,
--
Shigeru Hanada
