Thread: fdw validation function vs zero catalog id
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');
Martin Pihlak <martin.pihlak@gmail.com> writes:
> 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.
According to what? I can't find any documentation whatsoever on what
arguments that function is supposed to get.
regards, tom lane
Tom Lane wrote: > According to what? I can't find any documentation whatsoever on what > arguments that function is supposed to get. > > regards, tom lane > According to http://www.postgresql.org/docs/8.4/static/sql-createforeigndatawrapper.html: "The validator function must take two arguments: one of type text[], which will contain the array of options as stored in the system catalogs, and one of type oid, which will be the OID of the system catalog containing the options, or zero if the context is not known." regards, Martin
Martin Pihlak <martin.pihlak@gmail.com> writes: > Tom Lane wrote: >> According to what? I can't find any documentation whatsoever on what >> arguments that function is supposed to get. > According to > http://www.postgresql.org/docs/8.4/static/sql-createforeigndatawrapper.html: > "The validator function must take two arguments: one of type text[], which > will contain the array of options as stored in the system catalogs, and one > of type oid, which will be the OID of the system catalog containing the > options, or zero if the context is not known." Hmm, dunno how I missed that. But anyway ISTM the current code conforms to that specification just fine. I think what you're really lobbying for is that we remove the "or zero" escape hatch and insist that the backend code do whatever it has to do to supply a correct OID. This patch shows that that's not too hard right now, but are there going to be future situations where it's harder? What was the motivation for including the escape hatch in the first place? regards, tom lane
Tom Lane wrote: >> "The validator function must take two arguments: one of type text[], which >> will contain the array of options as stored in the system catalogs, and one >> of type oid, which will be the OID of the system catalog containing the >> options, or zero if the context is not known." > > Hmm, dunno how I missed that. But anyway ISTM the current code conforms > to that specification just fine. I think what you're really lobbying It certainly looks like a bug to me -- while performing CREATE or ALTER on a SQL/MED object, the catalog must surely be known, and one would expect that the validator function is passed the actual catalog id. Otherwise there would be no point for the validator function to accept the catalog id at all. > for is that we remove the "or zero" escape hatch and insist that the > backend code do whatever it has to do to supply a correct OID. This > patch shows that that's not too hard right now, but are there going to > be future situations where it's harder? What was the motivation for > including the escape hatch in the first place? > 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". regards, Martin
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');
Martin Pihlak wrote: > 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. Committed. I don't foresee any scenario either where we wouldn't know the catalog ID. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com