I wrote:
> The validator is run for the generic options specified to CREATE/ALTER FDW,
> SERVER and USER MAPPING (+ possible future SQL/MED objects). In this case the
> catalog is always known. Also we can assume that the catalog is known, if a
> user
> runs the validator directly. So yes, AFAICS there is no case for the "or
> zero".
>
Updated patch attached. This now also removes the "or zero" note from
the documentation and modifies postgresql_fdw_validator() to assume that
a valid catalog oid is always passed.
regards,
Martin
*** a/doc/src/sgml/ref/create_foreign_data_wrapper.sgml
--- b/doc/src/sgml/ref/create_foreign_data_wrapper.sgml
***************
*** 74,83 **** CREATE FOREIGN DATA WRAPPER <replaceable class="parameter">name</replaceable>
take two arguments: one of type <type>text[]</type>, which will
contain the array of options as stored in the system catalogs,
and one of type <type>oid</type>, which will be the OID of the
! system catalog containing the options, or zero if the context is
! not known. The return type is ignored; the function should
! indicate invalid options using
! the <function>ereport()</function> function.
</para>
</listitem>
</varlistentry>
--- 74,82 ----
take two arguments: one of type <type>text[]</type>, which will
contain the array of options as stored in the system catalogs,
and one of type <type>oid</type>, which will be the OID of the
! system catalog containing the options. The return type is ignored;
! the function should indicate invalid options using the
! <function>ereport()</function> function.
</para>
</listitem>
</varlistentry>
*** 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/backend/foreign/foreign.c
--- b/src/backend/foreign/foreign.c
***************
*** 372,378 **** is_conninfo_option(const char *option, Oid context)
struct ConnectionOption *opt;
for (opt = libpq_conninfo_options; opt->optname; opt++)
! if ((context == opt->optcontext || context == InvalidOid) && strcmp(opt->optname, option) == 0)
return true;
return false;
}
--- 372,378 ----
struct ConnectionOption *opt;
for (opt = libpq_conninfo_options; opt->optname; opt++)
! if (context == opt->optcontext && strcmp(opt->optname, option) == 0)
return true;
return false;
}
***************
*** 409,415 **** postgresql_fdw_validator(PG_FUNCTION_ARGS)
*/
initStringInfo(&buf);
for (opt = libpq_conninfo_options; opt->optname; opt++)
! if (catalog == InvalidOid || catalog == opt->optcontext)
appendStringInfo(&buf, "%s%s", (buf.len > 0) ? ", " : "",
opt->optname);
--- 409,415 ----
*/
initStringInfo(&buf);
for (opt = libpq_conninfo_options; opt->optname; opt++)
! if (catalog == opt->optcontext)
appendStringInfo(&buf, "%s%s", (buf.len > 0) ? ", " : "",
opt->optname);
*** 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 ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
