It appears that the function for validating generic options to a FDW,
SERVER and USER MAPPING is always passed a catalog oid of 0. Whereas
it should actually be passed the oid of the catalog that the options
apply to.
Attached patch fixes the issue by passing the proper catalog id from
transformGenericOptions().
PS. I personally would like this applied to 8.4 as well -- it'd enable
us to write a proper validator for pl/proxy fdw.
regards,
Martin
*** a/src/backend/commands/foreigncmds.c
--- b/src/backend/commands/foreigncmds.c
***************
*** 88,94 **** optionListToArray(List *options)
* This is used by CREATE/ALTER of FOREIGN DATA WRAPPER/SERVER/USER MAPPING.
*/
static Datum
! transformGenericOptions(Datum oldOptions,
List *options,
Oid fdwvalidator)
{
--- 88,95 ----
* This is used by CREATE/ALTER of FOREIGN DATA WRAPPER/SERVER/USER MAPPING.
*/
static Datum
! transformGenericOptions(Oid catalogId,
! Datum oldOptions,
List *options,
Oid fdwvalidator)
{
***************
*** 162,168 **** transformGenericOptions(Datum oldOptions,
result = optionListToArray(resultOptions);
if (fdwvalidator)
! OidFunctionCall2(fdwvalidator, result, (Datum) 0);
return result;
}
--- 163,169 ----
result = optionListToArray(resultOptions);
if (fdwvalidator)
! OidFunctionCall2(fdwvalidator, result, ObjectIdGetDatum(catalogId));
return result;
}
***************
*** 384,390 **** CreateForeignDataWrapper(CreateFdwStmt *stmt)
nulls[Anum_pg_foreign_data_wrapper_fdwacl - 1] = true;
! fdwoptions = transformGenericOptions(PointerGetDatum(NULL), stmt->options,
fdwvalidator);
if (PointerIsValid(DatumGetPointer(fdwoptions)))
--- 385,393 ----
nulls[Anum_pg_foreign_data_wrapper_fdwacl - 1] = true;
! fdwoptions = transformGenericOptions(ForeignDataWrapperRelationId,
! PointerGetDatum(NULL),
! stmt->options,
fdwvalidator);
if (PointerIsValid(DatumGetPointer(fdwoptions)))
***************
*** 501,507 **** AlterForeignDataWrapper(AlterFdwStmt *stmt)
datum = PointerGetDatum(NULL);
/* Transform the options */
! datum = transformGenericOptions(datum, stmt->options, fdwvalidator);
if (PointerIsValid(DatumGetPointer(datum)))
repl_val[Anum_pg_foreign_data_wrapper_fdwoptions - 1] = datum;
--- 504,513 ----
datum = PointerGetDatum(NULL);
/* Transform the options */
! datum = transformGenericOptions(ForeignDataWrapperRelationId,
! datum,
! stmt->options,
! fdwvalidator);
if (PointerIsValid(DatumGetPointer(datum)))
repl_val[Anum_pg_foreign_data_wrapper_fdwoptions - 1] = datum;
***************
*** 667,673 **** CreateForeignServer(CreateForeignServerStmt *stmt)
nulls[Anum_pg_foreign_server_srvacl - 1] = true;
/* Add server options */
! srvoptions = transformGenericOptions(PointerGetDatum(NULL), stmt->options,
fdw->fdwvalidator);
if (PointerIsValid(DatumGetPointer(srvoptions)))
--- 673,681 ----
nulls[Anum_pg_foreign_server_srvacl - 1] = true;
/* Add server options */
! srvoptions = transformGenericOptions(ForeignServerRelationId,
! PointerGetDatum(NULL),
! stmt->options,
fdw->fdwvalidator);
if (PointerIsValid(DatumGetPointer(srvoptions)))
***************
*** 765,771 **** AlterForeignServer(AlterForeignServerStmt *stmt)
datum = PointerGetDatum(NULL);
/* Prepare the options array */
! datum = transformGenericOptions(datum, stmt->options,
fdw->fdwvalidator);
if (PointerIsValid(DatumGetPointer(datum)))
--- 773,781 ----
datum = PointerGetDatum(NULL);
/* Prepare the options array */
! datum = transformGenericOptions(ForeignServerRelationId,
! datum,
! stmt->options,
fdw->fdwvalidator);
if (PointerIsValid(DatumGetPointer(datum)))
***************
*** 936,942 **** CreateUserMapping(CreateUserMappingStmt *stmt)
values[Anum_pg_user_mapping_umserver - 1] = ObjectIdGetDatum(srv->serverid);
/* Add user options */
! useoptions = transformGenericOptions(PointerGetDatum(NULL), stmt->options,
fdw->fdwvalidator);
if (PointerIsValid(DatumGetPointer(useoptions)))
--- 946,954 ----
values[Anum_pg_user_mapping_umserver - 1] = ObjectIdGetDatum(srv->serverid);
/* Add user options */
! useoptions = transformGenericOptions(UserMappingRelationId,
! PointerGetDatum(NULL),
! stmt->options,
fdw->fdwvalidator);
if (PointerIsValid(DatumGetPointer(useoptions)))
***************
*** 1031,1037 **** AlterUserMapping(AlterUserMappingStmt *stmt)
datum = PointerGetDatum(NULL);
/* Prepare the options array */
! datum = transformGenericOptions(datum, stmt->options,
fdw->fdwvalidator);
if (PointerIsValid(DatumGetPointer(datum)))
--- 1043,1051 ----
datum = PointerGetDatum(NULL);
/* Prepare the options array */
! datum = transformGenericOptions(UserMappingRelationId,
! datum,
! stmt->options,
fdw->fdwvalidator);
if (PointerIsValid(DatumGetPointer(datum)))
*** a/src/test/regress/expected/foreign_data.out
--- b/src/test/regress/expected/foreign_data.out
***************
*** 284,290 **** CREATE SERVER s6 VERSION '16.0' FOREIGN DATA WRAPPER foo OPTIONS (host 'a', dbna
CREATE SERVER s7 TYPE 'oracle' VERSION '17.0' FOREIGN DATA WRAPPER foo OPTIONS (host 'a', dbname 'b');
CREATE SERVER s8 FOREIGN DATA WRAPPER postgresql OPTIONS (foo '1'); -- ERROR
ERROR: invalid option "foo"
! HINT: Valid options in this context are: authtype, service, user, password, connect_timeout, dbname, host, hostaddr, port, tty, options, requiressl, sslmode, gsslib
CREATE SERVER s8 FOREIGN DATA WRAPPER postgresql OPTIONS (host 'localhost', dbname 's8db');
\des+
List of foreign servers
--- 284,290 ----
CREATE SERVER s7 TYPE 'oracle' VERSION '17.0' FOREIGN DATA WRAPPER foo OPTIONS (host 'a', dbname 'b');
CREATE SERVER s8 FOREIGN DATA WRAPPER postgresql OPTIONS (foo '1'); -- ERROR
ERROR: invalid option "foo"
! HINT: Valid options in this context are: authtype, service, connect_timeout, dbname, host, hostaddr, port, tty, options, requiressl, sslmode, gsslib
CREATE SERVER s8 FOREIGN DATA WRAPPER postgresql OPTIONS (host 'localhost', dbname 's8db');
\des+
List of foreign servers
***************
*** 395,401 **** ERROR: permission denied for foreign-data wrapper foo
RESET ROLE;
ALTER SERVER s8 OPTIONS (foo '1'); -- ERROR option validation
ERROR: invalid option "foo"
! HINT: Valid options in this context are: authtype, service, user, password, connect_timeout, dbname, host, hostaddr, port, tty, options, requiressl, sslmode, gsslib
ALTER SERVER s8 OPTIONS (connect_timeout '30', SET dbname 'db1', DROP host);
SET ROLE regress_test_role;
ALTER SERVER s1 OWNER TO regress_test_indirect; -- ERROR
--- 395,401 ----
RESET ROLE;
ALTER SERVER s8 OPTIONS (foo '1'); -- ERROR option validation
ERROR: invalid option "foo"
! HINT: Valid options in this context are: authtype, service, connect_timeout, dbname, host, hostaddr, port, tty, options, requiressl, sslmode, gsslib
ALTER SERVER s8 OPTIONS (connect_timeout '30', SET dbname 'db1', DROP host);
SET ROLE regress_test_role;
ALTER SERVER s1 OWNER TO regress_test_indirect; -- ERROR
***************
*** 534,540 **** ERROR: user mapping "foreign_data_user" already exists for server s4
CREATE USER MAPPING FOR public SERVER s4 OPTIONS (mapping 'is public');
CREATE USER MAPPING FOR user SERVER s8 OPTIONS (username 'test', password 'secret'); -- ERROR
ERROR: invalid option "username"
! HINT: Valid options in this context are: authtype, service, user, password, connect_timeout, dbname, host, hostaddr, port, tty, options, requiressl, sslmode, gsslib
CREATE USER MAPPING FOR user SERVER s8 OPTIONS (user 'test', password 'secret');
ALTER SERVER s5 OWNER TO regress_test_role;
ALTER SERVER s6 OWNER TO regress_test_indirect;
--- 534,540 ----
CREATE USER MAPPING FOR public SERVER s4 OPTIONS (mapping 'is public');
CREATE USER MAPPING FOR user SERVER s8 OPTIONS (username 'test', password 'secret'); -- ERROR
ERROR: invalid option "username"
! HINT: Valid options in this context are: user, password
CREATE USER MAPPING FOR user SERVER s8 OPTIONS (user 'test', password 'secret');
ALTER SERVER s5 OWNER TO regress_test_role;
ALTER SERVER s6 OWNER TO regress_test_indirect;
***************
*** 573,579 **** ALTER USER MAPPING FOR public SERVER s5 OPTIONS (gotcha 'true'); -- E
ERROR: user mapping "public" does not exist for the server
ALTER USER MAPPING FOR current_user SERVER s8 OPTIONS (username 'test'); -- ERROR
ERROR: invalid option "username"
! HINT: Valid options in this context are: authtype, service, user, password, connect_timeout, dbname, host, hostaddr, port, tty, options, requiressl, sslmode, gsslib
ALTER USER MAPPING FOR current_user SERVER s8 OPTIONS (DROP user, SET password 'public');
SET ROLE regress_test_role;
ALTER USER MAPPING FOR current_user SERVER s5 OPTIONS (ADD modified '1');
--- 573,579 ----
ERROR: user mapping "public" does not exist for the server
ALTER USER MAPPING FOR current_user SERVER s8 OPTIONS (username 'test'); -- ERROR
ERROR: invalid option "username"
! HINT: Valid options in this context are: user, password
ALTER USER MAPPING FOR current_user SERVER s8 OPTIONS (DROP user, SET password 'public');
SET ROLE regress_test_role;
ALTER USER MAPPING FOR current_user SERVER s5 OPTIONS (ADD modified '1');
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
