(2012/03/01 0:33), Tom Lane wrote: > I don't think that creating such a dependency is acceptable. > Even if we didn't mind the dependency, you said yourself that > contrib/postgresql_fdw's validator will accept stuff that's not > appropriate for dblink.
Agreed. I think that these two contrib modules (and all FDW modules) should have individual validator for each to avoid undesirable dependency and naming conflict, and such validator function should be inside each module, but not in core. How about moving postgresql_fdw_validator into dblink, with renaming to dblink_fdw_validator? Attached patch achieves such changes. I've left postgresql_fdw_validator" in foreign_data regression test section, so that foreign_data section can still check whether FDW DDLs invoke validator function. I used the name "postgresql_fdw_validator" for test validator to make change as little as possible. This change requires dblink to have new function, so its version should be bumped to 1.1. These changes have no direct relation to PostgreSQL FDW, so this patch can be applied by itself. If this patch has been applied, I'll rename pgsql_fdw to postgresql_fdw which contains product name fully spelled out. -- Shigeru Hanada
diff --git a/contrib/dblink/Makefile b/contrib/dblink/Makefile index ac63748..a27db88 100644 *** a/contrib/dblink/Makefile --- b/contrib/dblink/Makefile *************** SHLIB_LINK = $(libpq) *** 7,13 **** SHLIB_PREREQS = submake-libpq EXTENSION = dblink ! DATA = dblink--1.0.sql dblink--unpackaged--1.0.sql REGRESS = dblink --- 7,13 ---- SHLIB_PREREQS = submake-libpq EXTENSION = dblink ! DATA = dblink--1.1.sql dblink--1.0--1.1.sql dblink--unpackaged--1.0.sql REGRESS = dblink diff --git a/contrib/dblink/dblink--1.0--1.1.sql b/contrib/dblink/dblink--1.0--1.1.sql index ...3dd02a0 . *** a/contrib/dblink/dblink--1.0--1.1.sql --- b/contrib/dblink/dblink--1.0--1.1.sql *************** *** 0 **** --- 1,17 ---- + /* contrib/dblink/dblink--1.0--1.1.sql */ + + -- complain if script is sourced in psql, rather than via ALTER EXTENSION + \echo Use "ALTER EXTENSION dblink UPDATE" to load this file. \quit + + /* First we have to create validator function */ + CREATE FUNCTION dblink_fdw_validator( + options text[], + catalog oid + ) + RETURNS boolean + AS 'MODULE_PATHNAME', 'dblink_fdw_validator' + LANGUAGE C STRICT; + + /* Then we add the validator */ + ALTER EXTENSION dblink ADD FUNCTION dblink_fdw_validator(text[], oid); + diff --git a/contrib/dblink/dblink--1.1.sql b/contrib/dblink/dblink--1.1.sql index ...ec02498 . *** a/contrib/dblink/dblink--1.1.sql --- b/contrib/dblink/dblink--1.1.sql *************** *** 0 **** --- 1,231 ---- + /* contrib/dblink/dblink--1.1.sql */ + + -- complain if script is sourced in psql, rather than via CREATE EXTENSION + \echo Use "CREATE EXTENSION dblink" to load this file. \quit + + -- dblink_connect now restricts non-superusers to password + -- authenticated connections + CREATE FUNCTION dblink_connect (text) + RETURNS text + AS 'MODULE_PATHNAME','dblink_connect' + LANGUAGE C STRICT; + + CREATE FUNCTION dblink_connect (text, text) + RETURNS text + AS 'MODULE_PATHNAME','dblink_connect' + LANGUAGE C STRICT; + + -- dblink_connect_u allows non-superusers to use + -- non-password authenticated connections, but initially + -- privileges are revoked from public + CREATE FUNCTION dblink_connect_u (text) + RETURNS text + AS 'MODULE_PATHNAME','dblink_connect' + LANGUAGE C STRICT SECURITY DEFINER; + + CREATE FUNCTION dblink_connect_u (text, text) + RETURNS text + AS 'MODULE_PATHNAME','dblink_connect' + LANGUAGE C STRICT SECURITY DEFINER; + + REVOKE ALL ON FUNCTION dblink_connect_u (text) FROM public; + REVOKE ALL ON FUNCTION dblink_connect_u (text, text) FROM public; + + CREATE FUNCTION dblink_disconnect () + RETURNS text + AS 'MODULE_PATHNAME','dblink_disconnect' + LANGUAGE C STRICT; + + CREATE FUNCTION dblink_disconnect (text) + RETURNS text + AS 'MODULE_PATHNAME','dblink_disconnect' + LANGUAGE C STRICT; + + CREATE FUNCTION dblink_open (text, text) + RETURNS text + AS 'MODULE_PATHNAME','dblink_open' + LANGUAGE C STRICT; + + CREATE FUNCTION dblink_open (text, text, boolean) + RETURNS text + AS 'MODULE_PATHNAME','dblink_open' + LANGUAGE C STRICT; + + CREATE FUNCTION dblink_open (text, text, text) + RETURNS text + AS 'MODULE_PATHNAME','dblink_open' + LANGUAGE C STRICT; + + CREATE FUNCTION dblink_open (text, text, text, boolean) + RETURNS text + AS 'MODULE_PATHNAME','dblink_open' + LANGUAGE C STRICT; + + CREATE FUNCTION dblink_fetch (text, int) + RETURNS setof record + AS 'MODULE_PATHNAME','dblink_fetch' + LANGUAGE C STRICT; + + CREATE FUNCTION dblink_fetch (text, int, boolean) + RETURNS setof record + AS 'MODULE_PATHNAME','dblink_fetch' + LANGUAGE C STRICT; + + CREATE FUNCTION dblink_fetch (text, text, int) + RETURNS setof record + AS 'MODULE_PATHNAME','dblink_fetch' + LANGUAGE C STRICT; + + CREATE FUNCTION dblink_fetch (text, text, int, boolean) + RETURNS setof record + AS 'MODULE_PATHNAME','dblink_fetch' + LANGUAGE C STRICT; + + CREATE FUNCTION dblink_close (text) + RETURNS text + AS 'MODULE_PATHNAME','dblink_close' + LANGUAGE C STRICT; + + CREATE FUNCTION dblink_close (text, boolean) + RETURNS text + AS 'MODULE_PATHNAME','dblink_close' + LANGUAGE C STRICT; + + CREATE FUNCTION dblink_close (text, text) + RETURNS text + AS 'MODULE_PATHNAME','dblink_close' + LANGUAGE C STRICT; + + CREATE FUNCTION dblink_close (text, text, boolean) + RETURNS text + AS 'MODULE_PATHNAME','dblink_close' + LANGUAGE C STRICT; + + CREATE FUNCTION dblink (text, text) + RETURNS setof record + AS 'MODULE_PATHNAME','dblink_record' + LANGUAGE C STRICT; + + CREATE FUNCTION dblink (text, text, boolean) + RETURNS setof record + AS 'MODULE_PATHNAME','dblink_record' + LANGUAGE C STRICT; + + CREATE FUNCTION dblink (text) + RETURNS setof record + AS 'MODULE_PATHNAME','dblink_record' + LANGUAGE C STRICT; + + CREATE FUNCTION dblink (text, boolean) + RETURNS setof record + AS 'MODULE_PATHNAME','dblink_record' + LANGUAGE C STRICT; + + CREATE FUNCTION dblink_exec (text, text) + RETURNS text + AS 'MODULE_PATHNAME','dblink_exec' + LANGUAGE C STRICT; + + CREATE FUNCTION dblink_exec (text, text, boolean) + RETURNS text + AS 'MODULE_PATHNAME','dblink_exec' + LANGUAGE C STRICT; + + CREATE FUNCTION dblink_exec (text) + RETURNS text + AS 'MODULE_PATHNAME','dblink_exec' + LANGUAGE C STRICT; + + CREATE FUNCTION dblink_exec (text,boolean) + RETURNS text + AS 'MODULE_PATHNAME','dblink_exec' + LANGUAGE C STRICT; + + CREATE TYPE dblink_pkey_results AS (position int, colname text); + + CREATE FUNCTION dblink_get_pkey (text) + RETURNS setof dblink_pkey_results + AS 'MODULE_PATHNAME','dblink_get_pkey' + LANGUAGE C STRICT; + + CREATE FUNCTION dblink_build_sql_insert (text, int2vector, int, _text, _text) + RETURNS text + AS 'MODULE_PATHNAME','dblink_build_sql_insert' + LANGUAGE C STRICT; + + CREATE FUNCTION dblink_build_sql_delete (text, int2vector, int, _text) + RETURNS text + AS 'MODULE_PATHNAME','dblink_build_sql_delete' + LANGUAGE C STRICT; + + CREATE FUNCTION dblink_build_sql_update (text, int2vector, int, _text, _text) + RETURNS text + AS 'MODULE_PATHNAME','dblink_build_sql_update' + LANGUAGE C STRICT; + + CREATE FUNCTION dblink_current_query () + RETURNS text + AS 'MODULE_PATHNAME','dblink_current_query' + LANGUAGE C; + + CREATE FUNCTION dblink_send_query(text, text) + RETURNS int4 + AS 'MODULE_PATHNAME', 'dblink_send_query' + LANGUAGE C STRICT; + + CREATE FUNCTION dblink_is_busy(text) + RETURNS int4 + AS 'MODULE_PATHNAME', 'dblink_is_busy' + LANGUAGE C STRICT; + + CREATE FUNCTION dblink_get_result(text) + RETURNS SETOF record + AS 'MODULE_PATHNAME', 'dblink_get_result' + LANGUAGE C STRICT; + + CREATE FUNCTION dblink_get_result(text, bool) + RETURNS SETOF record + AS 'MODULE_PATHNAME', 'dblink_get_result' + LANGUAGE C STRICT; + + CREATE FUNCTION dblink_get_connections() + RETURNS text[] + AS 'MODULE_PATHNAME', 'dblink_get_connections' + LANGUAGE C; + + CREATE FUNCTION dblink_cancel_query(text) + RETURNS text + AS 'MODULE_PATHNAME', 'dblink_cancel_query' + LANGUAGE C STRICT; + + CREATE FUNCTION dblink_error_message(text) + RETURNS text + AS 'MODULE_PATHNAME', 'dblink_error_message' + LANGUAGE C STRICT; + + CREATE FUNCTION dblink_get_notify( + OUT notify_name TEXT, + OUT be_pid INT4, + OUT extra TEXT + ) + RETURNS setof record + AS 'MODULE_PATHNAME', 'dblink_get_notify' + LANGUAGE C STRICT; + + CREATE FUNCTION dblink_get_notify( + conname TEXT, + OUT notify_name TEXT, + OUT be_pid INT4, + OUT extra TEXT + ) + RETURNS setof record + AS 'MODULE_PATHNAME', 'dblink_get_notify' + LANGUAGE C STRICT; + + CREATE FUNCTION dblink_fdw_validator( + options text[], + catalog oid + ) + RETURNS boolean + AS 'MODULE_PATHNAME', 'dblink_fdw_validator' + LANGUAGE C STRICT; diff --git a/contrib/dblink/dblink.c b/contrib/dblink/dblink.c index 36a8e3e..1687c53 100644 *** a/contrib/dblink/dblink.c --- b/contrib/dblink/dblink.c *************** *** 36,43 **** --- 36,46 ---- #include "libpq-fe.h" #include "funcapi.h" + #include "access/reloptions.h" #include "catalog/indexing.h" #include "catalog/namespace.h" + #include "catalog/pg_foreign_server.h" + #include "catalog/pg_user_mapping.h" #include "catalog/pg_type.h" #include "executor/spi.h" #include "foreign/foreign.h" *************** validate_pkattnums(Relation rel, *** 2411,2413 **** --- 2414,2516 ---- errmsg("invalid attribute number %d", pkattnum))); } } + + + /* + * Describes the valid options for postgresql FDW, server, and user mapping. + */ + struct ConnectionOption + { + const char *optname; + Oid optcontext; /* Oid of catalog in which option may appear */ + }; + + /* + * Copied from fe-connect.c PQconninfoOptions. + * + * The list is small - don't bother with bsearch if it stays so. + */ + static struct ConnectionOption libpq_conninfo_options[] = { + {"authtype", ForeignServerRelationId}, + {"service", ForeignServerRelationId}, + {"user", UserMappingRelationId}, + {"password", UserMappingRelationId}, + {"connect_timeout", ForeignServerRelationId}, + {"dbname", ForeignServerRelationId}, + {"host", ForeignServerRelationId}, + {"hostaddr", ForeignServerRelationId}, + {"port", ForeignServerRelationId}, + {"tty", ForeignServerRelationId}, + {"options", ForeignServerRelationId}, + {"requiressl", ForeignServerRelationId}, + {"sslmode", ForeignServerRelationId}, + {"gsslib", ForeignServerRelationId}, + {NULL, InvalidOid} + }; + + + /* + * Check if the provided option is one of libpq conninfo options. + * context is the Oid of the catalog the option came from, or 0 if we + * don't care. + */ + static bool + is_conninfo_option(const char *option, Oid context) + { + struct ConnectionOption *opt; + + for (opt = libpq_conninfo_options; opt->optname; opt++) + if (context == opt->optcontext && strcmp(opt->optname, option) == 0) + return true; + return false; + } + + + /* + * Validate the generic option given to SERVER or USER MAPPING. + * Raise an ERROR if the option or its value is considered + * invalid. + * + * Valid server options are all libpq conninfo options except + * user and password -- these may only appear in USER MAPPING options. + */ + PG_FUNCTION_INFO_V1(dblink_fdw_validator); + Datum + dblink_fdw_validator(PG_FUNCTION_ARGS) + { + List *options_list = untransformRelOptions(PG_GETARG_DATUM(0)); + Oid catalog = PG_GETARG_OID(1); + + ListCell *cell; + + foreach(cell, options_list) + { + DefElem *def = lfirst(cell); + + if (!is_conninfo_option(def->defname, catalog)) + { + struct ConnectionOption *opt; + StringInfoData buf; + + /* + * Unknown option specified, complain about it. Provide a hint + * with list of valid options for the object. + */ + initStringInfo(&buf); + for (opt = libpq_conninfo_options; opt->optname; opt++) + if (catalog == opt->optcontext) + appendStringInfo(&buf, "%s%s", (buf.len > 0) ? ", " : "", + opt->optname); + + ereport(ERROR, + (errcode(ERRCODE_SYNTAX_ERROR), + errmsg("invalid option \"%s\"", def->defname), + errhint("Valid options in this context are: %s", + buf.data))); + + PG_RETURN_BOOL(false); + } + } + + PG_RETURN_BOOL(true); + } diff --git a/contrib/dblink/dblink.control b/contrib/dblink/dblink.control index 4333a9b..39f439a 100644 *** a/contrib/dblink/dblink.control --- b/contrib/dblink/dblink.control *************** *** 1,5 **** # dblink extension comment = 'connect to other PostgreSQL databases from within a database' ! default_version = '1.0' module_pathname = '$libdir/dblink' relocatable = true --- 1,5 ---- # dblink extension comment = 'connect to other PostgreSQL databases from within a database' ! default_version = '1.1' module_pathname = '$libdir/dblink' relocatable = true diff --git a/contrib/dblink/dblink.h b/contrib/dblink/dblink.h index 935d283..fd11658 100644 *** a/contrib/dblink/dblink.h --- b/contrib/dblink/dblink.h *************** extern Datum dblink_build_sql_delete(PG_ *** 58,62 **** --- 58,63 ---- extern Datum dblink_build_sql_update(PG_FUNCTION_ARGS); extern Datum dblink_current_query(PG_FUNCTION_ARGS); extern Datum dblink_get_notify(PG_FUNCTION_ARGS); + extern Datum dblink_fdw_validator(PG_FUNCTION_ARGS); #endif /* DBLINK_H */ diff --git a/contrib/dblink/expected/dblink.out b/contrib/dblink/expected/dblink.out index 511dd5e..0605354 100644 *** a/contrib/dblink/expected/dblink.out --- b/contrib/dblink/expected/dblink.out *************** SELECT dblink_disconnect('dtest1'); *** 785,792 **** -- test foreign data wrapper functionality CREATE USER dblink_regression_test; ! CREATE FOREIGN DATA WRAPPER postgresql; ! CREATE SERVER fdtest FOREIGN DATA WRAPPER postgresql OPTIONS (dbname 'contrib_regression'); CREATE USER MAPPING FOR public SERVER fdtest; GRANT USAGE ON FOREIGN SERVER fdtest TO dblink_regression_test; GRANT EXECUTE ON FUNCTION dblink_connect_u(text, text) TO dblink_regression_test; --- 785,792 ---- -- test foreign data wrapper functionality CREATE USER dblink_regression_test; ! CREATE FOREIGN DATA WRAPPER dblink_fdw VALIDATOR dblink_fdw_validator; ! CREATE SERVER fdtest FOREIGN DATA WRAPPER dblink_fdw OPTIONS (dbname 'contrib_regression'); CREATE USER MAPPING FOR public SERVER fdtest; GRANT USAGE ON FOREIGN SERVER fdtest TO dblink_regression_test; GRANT EXECUTE ON FUNCTION dblink_connect_u(text, text) TO dblink_regression_test; *************** REVOKE EXECUTE ON FUNCTION dblink_connec *** 825,831 **** DROP USER dblink_regression_test; DROP USER MAPPING FOR public SERVER fdtest; DROP SERVER fdtest; ! DROP FOREIGN DATA WRAPPER postgresql; -- test asynchronous notifications SELECT dblink_connect('dbname=contrib_regression'); dblink_connect --- 825,831 ---- DROP USER dblink_regression_test; DROP USER MAPPING FOR public SERVER fdtest; DROP SERVER fdtest; ! DROP FOREIGN DATA WRAPPER dblink_fdw; -- test asynchronous notifications SELECT dblink_connect('dbname=contrib_regression'); dblink_connect diff --git a/contrib/dblink/sql/dblink.sql b/contrib/dblink/sql/dblink.sql index 8c8ffe2..288ebf8 100644 *** a/contrib/dblink/sql/dblink.sql --- b/contrib/dblink/sql/dblink.sql *************** SELECT dblink_disconnect('dtest1'); *** 361,368 **** -- test foreign data wrapper functionality CREATE USER dblink_regression_test; ! CREATE FOREIGN DATA WRAPPER postgresql; ! CREATE SERVER fdtest FOREIGN DATA WRAPPER postgresql OPTIONS (dbname 'contrib_regression'); CREATE USER MAPPING FOR public SERVER fdtest; GRANT USAGE ON FOREIGN SERVER fdtest TO dblink_regression_test; GRANT EXECUTE ON FUNCTION dblink_connect_u(text, text) TO dblink_regression_test; --- 361,368 ---- -- test foreign data wrapper functionality CREATE USER dblink_regression_test; ! CREATE FOREIGN DATA WRAPPER dblink_fdw VALIDATOR dblink_fdw_validator; ! CREATE SERVER fdtest FOREIGN DATA WRAPPER dblink_fdw OPTIONS (dbname 'contrib_regression'); CREATE USER MAPPING FOR public SERVER fdtest; GRANT USAGE ON FOREIGN SERVER fdtest TO dblink_regression_test; GRANT EXECUTE ON FUNCTION dblink_connect_u(text, text) TO dblink_regression_test; *************** REVOKE EXECUTE ON FUNCTION dblink_connec *** 381,387 **** DROP USER dblink_regression_test; DROP USER MAPPING FOR public SERVER fdtest; DROP SERVER fdtest; ! DROP FOREIGN DATA WRAPPER postgresql; -- test asynchronous notifications SELECT dblink_connect('dbname=contrib_regression'); --- 381,387 ---- DROP USER dblink_regression_test; DROP USER MAPPING FOR public SERVER fdtest; DROP SERVER fdtest; ! DROP FOREIGN DATA WRAPPER dblink_fdw; -- test asynchronous notifications SELECT dblink_connect('dbname=contrib_regression'); diff --git a/doc/src/sgml/dblink.sgml b/doc/src/sgml/dblink.sgml index 855495c..613f447 100644 *** a/doc/src/sgml/dblink.sgml --- b/doc/src/sgml/dblink.sgml *************** dblink_connect(text connname, text conns *** 47,55 **** <para> The connection string may also be the name of an existing foreign server. It is recommended to use ! the <function>postgresql_fdw_validator</function> when defining ! the corresponding foreign-data wrapper. See the example below, as ! well as the following: <simplelist type="inline"> <member><xref linkend="sql-createforeigndatawrapper"></member> <member><xref linkend="sql-createserver"></member> --- 47,58 ---- <para> The connection string may also be the name of an existing foreign server. It is recommended to use ! the <function>dblink_fdw_validator</function> when defining ! the corresponding foreign-data wrapper. ! This validator function was added in <productname>PostgreSQL</> 9.2, and ! it has been called <function>postgresql_fdw_validator</function> in older ! versions. ! See the example below, as well as the following: <simplelist type="inline"> <member><xref linkend="sql-createforeigndatawrapper"></member> <member><xref linkend="sql-createserver"></member> *************** SELECT dblink_connect('myconn', 'dbname= *** 136,143 **** -- DETAIL: Non-superuser cannot connect if the server does not request a password. -- HINT: Target server's authentication method must be changed. CREATE USER dblink_regression_test WITH PASSWORD 'secret'; ! CREATE FOREIGN DATA WRAPPER postgresql VALIDATOR postgresql_fdw_validator; ! CREATE SERVER fdtest FOREIGN DATA WRAPPER postgresql OPTIONS (hostaddr '127.0.0.1', dbname 'contrib_regression'); CREATE USER MAPPING FOR dblink_regression_test SERVER fdtest OPTIONS (user 'dblink_regression_test', password 'secret'); GRANT USAGE ON FOREIGN SERVER fdtest TO dblink_regression_test; --- 139,146 ---- -- DETAIL: Non-superuser cannot connect if the server does not request a password. -- HINT: Target server's authentication method must be changed. CREATE USER dblink_regression_test WITH PASSWORD 'secret'; ! CREATE FOREIGN DATA WRAPPER dblink_fdw VALIDATOR dblink_fdw_validator; ! CREATE SERVER fdtest FOREIGN DATA WRAPPER dblink_fdw OPTIONS (hostaddr '127.0.0.1', dbname 'contrib_regression'); CREATE USER MAPPING FOR dblink_regression_test SERVER fdtest OPTIONS (user 'dblink_regression_test', password 'secret'); GRANT USAGE ON FOREIGN SERVER fdtest TO dblink_regression_test; *************** REVOKE SELECT ON TABLE foo FROM dblink_ *** 173,179 **** DROP USER MAPPING FOR dblink_regression_test SERVER fdtest; DROP USER dblink_regression_test; DROP SERVER fdtest; ! DROP FOREIGN DATA WRAPPER postgresql; </screen> </refsect1> </refentry> --- 176,182 ---- DROP USER MAPPING FOR dblink_regression_test SERVER fdtest; DROP USER dblink_regression_test; DROP SERVER fdtest; ! DROP FOREIGN DATA WRAPPER dblink_fdw; </screen> </refsect1> </refentry> diff --git a/doc/src/sgml/ref/create_foreign_data_wrapper.sgml b/doc/src/sgml/ref/create_foreign_data_wrapper.sgml index 804fb47..0d346fd 100644 *** a/doc/src/sgml/ref/create_foreign_data_wrapper.sgml --- b/doc/src/sgml/ref/create_foreign_data_wrapper.sgml *************** CREATE FOREIGN DATA WRAPPER <replaceable *** 123,133 **** </para> <para> ! There is one built-in foreign-data wrapper validator function ! provided: ! <filename>postgresql_fdw_validator</filename>, which accepts options corresponding to <application>libpq</> connection ! parameters. </para> </refsect1> --- 123,134 ---- </para> <para> ! Until <productname>PostgreSQL</productname> 9.1, there was one built-in ! foreign-data wrapper validator function provided: ! <function>postgresql_fdw_validator</function>, which accepts options corresponding to <application>libpq</> connection ! parameters. But it has been moved to <filename>contrib/dblink</> with ! renaming to <function>dblink_fdw_validator</function>. </para> </refsect1> diff --git a/src/backend/foreign/foreign.c b/src/backend/foreign/foreign.c index c4c2a61..6831c70 100644 *** a/src/backend/foreign/foreign.c --- b/src/backend/foreign/foreign.c *************** *** 25,31 **** extern Datum pg_options_to_table(PG_FUNCTION_ARGS); - extern Datum postgresql_fdw_validator(PG_FUNCTION_ARGS); --- 25,30 ---- *************** pg_options_to_table(PG_FUNCTION_ARGS) *** 401,504 **** /* - * Describes the valid options for postgresql FDW, server, and user mapping. - */ - struct ConnectionOption - { - const char *optname; - Oid optcontext; /* Oid of catalog in which option may appear */ - }; - - /* - * Copied from fe-connect.c PQconninfoOptions. - * - * The list is small - don't bother with bsearch if it stays so. - */ - static struct ConnectionOption libpq_conninfo_options[] = { - {"authtype", ForeignServerRelationId}, - {"service", ForeignServerRelationId}, - {"user", UserMappingRelationId}, - {"password", UserMappingRelationId}, - {"connect_timeout", ForeignServerRelationId}, - {"dbname", ForeignServerRelationId}, - {"host", ForeignServerRelationId}, - {"hostaddr", ForeignServerRelationId}, - {"port", ForeignServerRelationId}, - {"tty", ForeignServerRelationId}, - {"options", ForeignServerRelationId}, - {"requiressl", ForeignServerRelationId}, - {"sslmode", ForeignServerRelationId}, - {"gsslib", ForeignServerRelationId}, - {NULL, InvalidOid} - }; - - - /* - * Check if the provided option is one of libpq conninfo options. - * context is the Oid of the catalog the option came from, or 0 if we - * don't care. - */ - static bool - is_conninfo_option(const char *option, Oid context) - { - struct ConnectionOption *opt; - - for (opt = libpq_conninfo_options; opt->optname; opt++) - if (context == opt->optcontext && strcmp(opt->optname, option) == 0) - return true; - return false; - } - - - /* - * Validate the generic option given to SERVER or USER MAPPING. - * Raise an ERROR if the option or its value is considered - * invalid. - * - * Valid server options are all libpq conninfo options except - * user and password -- these may only appear in USER MAPPING options. - */ - Datum - postgresql_fdw_validator(PG_FUNCTION_ARGS) - { - List *options_list = untransformRelOptions(PG_GETARG_DATUM(0)); - Oid catalog = PG_GETARG_OID(1); - - ListCell *cell; - - foreach(cell, options_list) - { - DefElem *def = lfirst(cell); - - if (!is_conninfo_option(def->defname, catalog)) - { - struct ConnectionOption *opt; - StringInfoData buf; - - /* - * Unknown option specified, complain about it. Provide a hint - * with list of valid options for the object. - */ - initStringInfo(&buf); - for (opt = libpq_conninfo_options; opt->optname; opt++) - if (catalog == opt->optcontext) - appendStringInfo(&buf, "%s%s", (buf.len > 0) ? ", " : "", - opt->optname); - - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("invalid option \"%s\"", def->defname), - errhint("Valid options in this context are: %s", - buf.data))); - - PG_RETURN_BOOL(false); - } - } - - PG_RETURN_BOOL(true); - } - - /* * get_foreign_data_wrapper_oid - given a FDW name, look up the OID * * If missing_ok is false, throw an error if name not found. If true, just --- 400,405 ---- diff --git a/src/include/catalog/pg_proc.h b/src/include/catalog/pg_proc.h index 8700d0d..8970998 100644 *** a/src/include/catalog/pg_proc.h --- b/src/include/catalog/pg_proc.h *************** DESCR("filenode identifier of relation") *** 3399,3407 **** DATA(insert OID = 3034 ( pg_relation_filepath PGNSP PGUID 12 1 0 0 0 f f f f t f s 1 0 25 "2205" _null_ _null_ _null_ _null_ pg_relation_filepath _null_ _null_ _null_ )); DESCR("file path of relation"); - DATA(insert OID = 2316 ( postgresql_fdw_validator PGNSP PGUID 12 1 0 0 0 f f f f t f i 2 0 16 "1009 26" _null_ _null_ _null_ _null_ postgresql_fdw_validator _null_ _null_ _null_)); - DESCR("(internal)"); - DATA(insert OID = 2290 ( record_in PGNSP PGUID 12 1 0 0 0 f f f f t f s 3 0 2249 "2275 26 23" _null_ _null_ _null_ _null_ record_in _null_ _null_ _null_ )); DESCR("I/O"); DATA(insert OID = 2291 ( record_out PGNSP PGUID 12 1 0 0 0 f f f f t f s 1 0 2275 "2249" _null_ _null_ _null_ _null_ record_out _null_ _null_ _null_ )); --- 3399,3404 ---- diff --git a/src/test/regress/expected/foreign_data.out b/src/test/regress/expected/foreign_data.out index ba86883..161fbe7 100644 *** a/src/test/regress/expected/foreign_data.out --- b/src/test/regress/expected/foreign_data.out *************** CREATE ROLE regress_test_role2; *** 13,18 **** --- 13,33 ---- CREATE ROLE regress_test_role_super SUPERUSER; CREATE ROLE regress_test_indirect; CREATE ROLE unprivileged_role; + -- validator for this test + CREATE FUNCTION postgresql_fdw_validator(text[], oid) RETURNS boolean AS $$ + DECLARE + BEGIN + IF $1::text LIKE '%foo%' THEN + RAISE 'invalid option "foo"' + USING HINT = 'Valid options in this context are: authtype, service, connect_timeout, dbname, host, hostaddr, port, tty, options, requiressl, sslmode, gsslib'; + ELSIF $1::text LIKE '%username%' THEN + RAISE 'invalid option "username"' + USING HINT = 'Valid options in this context are: user, password'; + END IF; + + RETURN true; + END + $$ LANGUAGE plpgsql; CREATE FOREIGN DATA WRAPPER dummy; COMMENT ON FOREIGN DATA WRAPPER dummy IS 'useless'; CREATE FOREIGN DATA WRAPPER postgresql VALIDATOR postgresql_fdw_validator; *************** DROP FOREIGN DATA WRAPPER postgresql CAS *** 1201,1206 **** --- 1216,1222 ---- DROP FOREIGN DATA WRAPPER dummy CASCADE; NOTICE: drop cascades to server s0 \c + DROP FUNCTION postgresql_fdw_validator(text[], oid); DROP ROLE foreign_data_user; -- At this point we should have no wrappers, no servers, and no mappings. SELECT fdwname, fdwhandler, fdwvalidator, fdwoptions FROM pg_foreign_data_wrapper; diff --git a/src/test/regress/sql/foreign_data.sql b/src/test/regress/sql/foreign_data.sql index 0c95672..9cf9ca7 100644 *** a/src/test/regress/sql/foreign_data.sql --- b/src/test/regress/sql/foreign_data.sql *************** CREATE ROLE regress_test_role_super SUPE *** 20,25 **** --- 20,41 ---- CREATE ROLE regress_test_indirect; CREATE ROLE unprivileged_role; + -- validator for this test + CREATE FUNCTION postgresql_fdw_validator(text[], oid) RETURNS boolean AS $$ + DECLARE + BEGIN + IF $1::text LIKE '%foo%' THEN + RAISE 'invalid option "foo"' + USING HINT = 'Valid options in this context are: authtype, service, connect_timeout, dbname, host, hostaddr, port, tty, options, requiressl, sslmode, gsslib'; + ELSIF $1::text LIKE '%username%' THEN + RAISE 'invalid option "username"' + USING HINT = 'Valid options in this context are: user, password'; + END IF; + + RETURN true; + END + $$ LANGUAGE plpgsql; + CREATE FOREIGN DATA WRAPPER dummy; COMMENT ON FOREIGN DATA WRAPPER dummy IS 'useless'; CREATE FOREIGN DATA WRAPPER postgresql VALIDATOR postgresql_fdw_validator; *************** DROP ROLE regress_test_role2; *** 495,500 **** --- 511,517 ---- DROP FOREIGN DATA WRAPPER postgresql CASCADE; DROP FOREIGN DATA WRAPPER dummy CASCADE; \c + DROP FUNCTION postgresql_fdw_validator(text[], oid); DROP ROLE foreign_data_user; -- At this point we should have no wrappers, no servers, and no mappings.
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers