Обсуждение: fdw validation function vs zero catalog id

Поиск
Список
Период
Сортировка

fdw validation function vs zero catalog id

От
Martin Pihlak
Дата:
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');

Re: fdw validation function vs zero catalog id

От
Tom Lane
Дата:
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


Re: fdw validation function vs zero catalog id

От
Martin Pihlak
Дата:
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



Re: fdw validation function vs zero catalog id

От
Tom Lane
Дата:
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


Re: fdw validation function vs zero catalog id

От
Martin Pihlak
Дата:
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




Re: fdw validation function vs zero catalog id

От
Martin Pihlak
Дата:
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');

Re: fdw validation function vs zero catalog id

От
Heikki Linnakangas
Дата:
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