Re: Should we get rid of custom_variable_classes altogether? - Mailing list pgsql-hackers

Simon Riggs <simon@2ndQuadrant.com> writes:
> On Sun, Oct 2, 2011 at 10:05 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> So at this point I'd vote for just dropping it and always allowing
>> custom (that is, qualified) GUC names to be set, whether the prefix
>> corresponds to any loaded module or not.

> Sounds sensible. One less thing to configure is a good thing.

Attached is a draft patch for that.

While working on this I got annoyed at our cheesy handling of the
situation where a "placeholder" value has to be turned into a real
setting, which happens when the corresponding extension gets loaded.
There are several issues:

1. If it's a SUSET variable, a preceding attempt to set the value via
SET will fail even if you're a superuser, for example

regression=# set plpgsql.variable_conflict = use_variable;
SET
regression=# load 'plpgsql';
ERROR:  permission denied to set parameter "plpgsql.variable_conflict"

The reason for that is that define_custom_variable doesn't know whether
the pre-existing value was set by a superuser, so it must assume the
worst.  Seems like we could easily fix that by having set_config_option
set a flag in the GUC variable noting whether a SET was done by a
superuser or not.

2. If you do get an error while re-assigning the pre-existing value of
the variable, it's thrown as an ERROR.  This is really pretty nasty
because it'll abort execution of the extension module's init function;
for example, a likely consequence is that other custom variables of
the module don't set set up correctly, and it could easily be a lot
worse if there are other things the init function hasn't done yet.

I think we need to arrange that set_config_option only reports failures
to apply such values as WARNING, not ERROR.  There isn't anything in its
present API that could be used for that, but perhaps we could add a new
enum variant for "action" that commands it.

3. We aren't very careful about preserving the reset value of the
variable, in case it's different from the active value (which could
happen if the user did a SET and there's also a value from the
postgresql.conf file).

This seems like it just requires working a bit harder in
define_custom_variable, to reapply the reset value as well as the
current value of the variable.

Any objections to fixing that stuff, while I'm looking at it?

            regards, tom lane

diff --git a/doc/src/sgml/auth-delay.sgml b/doc/src/sgml/auth-delay.sgml
index e377c980cab919a407294356f27ede0255e63897..91549ffe4f61e019645866fcd98a6f2fb2f52998 100644
*** a/doc/src/sgml/auth-delay.sgml
--- b/doc/src/sgml/auth-delay.sgml
***************
*** 42,57 ****
    </variablelist>

    <para>
!    In order to set these parameters in your <filename>postgresql.conf</> file,
!    you will need to add <literal>auth_delay</> to
!    <xref linkend="guc-custom-variable-classes">.  Typical usage might be:
    </para>

  <programlisting>
  # postgresql.conf
  shared_preload_libraries = 'auth_delay'

- custom_variable_classes = 'auth_delay'
  auth_delay.milliseconds = '500'
  </programlisting>
   </sect2>
--- 42,55 ----
    </variablelist>

    <para>
!    These parameters must be set in <filename>postgresql.conf</>.
!    Typical usage might be:
    </para>

  <programlisting>
  # postgresql.conf
  shared_preload_libraries = 'auth_delay'

  auth_delay.milliseconds = '500'
  </programlisting>
   </sect2>
diff --git a/doc/src/sgml/auto-explain.sgml b/doc/src/sgml/auto-explain.sgml
index b16f9064ffc05d86637c054bc7b38221988b11db..6a8da566fbb200da0ab30f1cee5caee113645907 100644
*** a/doc/src/sgml/auto-explain.sgml
--- b/doc/src/sgml/auto-explain.sgml
*************** LOAD 'auto_explain';
*** 158,173 ****
    </variablelist>

    <para>
!    In order to set these parameters in your <filename>postgresql.conf</> file,
!    you will need to add <literal>auto_explain</> to
!    <xref linkend="guc-custom-variable-classes">.  Typical usage might be:
    </para>

  <programlisting>
  # postgresql.conf
  shared_preload_libraries = 'auto_explain'

- custom_variable_classes = 'auto_explain'
  auto_explain.log_min_duration = '3s'
  </programlisting>
   </sect2>
--- 158,171 ----
    </variablelist>

    <para>
!    These parameters must be set in <filename>postgresql.conf</>.
!    Typical usage might be:
    </para>

  <programlisting>
  # postgresql.conf
  shared_preload_libraries = 'auto_explain'

  auto_explain.log_min_duration = '3s'
  </programlisting>
   </sect2>
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 3282ab4f20303a986b6057c59a4bb979e20d497a..fbcd455694bfee1a17b8ebcc6d2ac504a09d0833 100644
*** a/doc/src/sgml/config.sgml
--- b/doc/src/sgml/config.sgml
*************** dynamic_library_path = 'C:\tools\postgre
*** 5940,5997 ****
      <para>
       This feature was designed to allow parameters not normally known to
       <productname>PostgreSQL</productname> to be added by add-on modules
!      (such as procedural languages).  This allows add-on modules to be
       configured in the standard ways.
      </para>

-     <variablelist>
-
-      <varlistentry id="guc-custom-variable-classes" xreflabel="custom_variable_classes">
-       <term><varname>custom_variable_classes</varname> (<type>string</type>)</term>
-       <indexterm>
-        <primary><varname>custom_variable_classes</> configuration parameter</primary>
-       </indexterm>
-       <listitem>
-        <para>
-         This variable specifies one or several class names to be used for
-         custom variables, in the form of a comma-separated list. A custom
-         variable is a variable not normally known
-         to <productname>PostgreSQL</productname> proper but used by some
-         add-on module.  Such variables must have names consisting of a class
-         name, a dot, and a variable name.  <varname>custom_variable_classes</>
-         specifies all the class names in use in a particular installation.
-         This parameter can only be set in the <filename>postgresql.conf</>
-         file or on the server command line.
-        </para>
-
-       </listitem>
-      </varlistentry>
-     </variablelist>
-
      <para>
!      The difficulty with setting custom variables in
!      <filename>postgresql.conf</> is that the file must be read before add-on
!      modules have been loaded, and so custom variables would ordinarily be
!      rejected as unknown.  When <varname>custom_variable_classes</> is set,
!      the server will accept definitions of arbitrary variables within each
!      specified class.  These variables will be treated as placeholders and
!      will have no function until the module that defines them is loaded. When a
!      module for a specific class is loaded, it will add the proper variable
!      definitions for its class name, convert any placeholder
!      values according to those definitions, and issue warnings for any
!      unrecognized placeholders of its class that remain.
      </para>

      <para>
!      Here is an example of what <filename>postgresql.conf</> might contain
!      when using custom variables:
!
! <programlisting>
! custom_variable_classes = 'plpgsql,plperl'
! plpgsql.variable_conflict = use_variable
! plperl.use_strict = true
! plruby.use_strict = true        # generates error: unknown class name
! </programlisting>
      </para>
     </sect1>

--- 5940,5964 ----
      <para>
       This feature was designed to allow parameters not normally known to
       <productname>PostgreSQL</productname> to be added by add-on modules
!      (such as procedural languages).  This allows extension modules to be
       configured in the standard ways.
      </para>

      <para>
!      Custom options have two-part names: an extension name, then a dot, then
!      the parameter name proper, much like qualified names in SQL.  An example
!      is <literal>plpgsql.variable_conflict</>.
      </para>

      <para>
!      Because custom options may need to be set in processes that have not
!      loaded the relevant extension module, <productname>PostgreSQL</>
!      will accept a setting for any two-part parameter name.  Such variables
!      are treated as placeholders and have no function until the module that
!      defines them is loaded. When an extension module is loaded, it will add
!      its variable definitions, convert any placeholder values according to
!      those definitions, and issue warnings for any unrecognized placeholders
!      that begin with its extension name.
      </para>
     </sect1>

diff --git a/doc/src/sgml/pgstatstatements.sgml b/doc/src/sgml/pgstatstatements.sgml
index 52268c545d74a5baf8c601705dec66d82711c8a6..5a0230c428618b53069f6b2f38e2d4acdcdaebff 100644
*** a/doc/src/sgml/pgstatstatements.sgml
--- b/doc/src/sgml/pgstatstatements.sgml
***************
*** 275,290 ****
    </para>

    <para>
!    In order to set any of these parameters in your
!    <filename>postgresql.conf</> file,
!    you will need to add <literal>pg_stat_statements</> to
!    <xref linkend="guc-custom-variable-classes">.  Typical usage might be:

  <programlisting>
  # postgresql.conf
  shared_preload_libraries = 'pg_stat_statements'

- custom_variable_classes = 'pg_stat_statements'
  pg_stat_statements.max = 10000
  pg_stat_statements.track = all
  </programlisting>
--- 275,287 ----
    </para>

    <para>
!    These parameters must be set in <filename>postgresql.conf</>.
!    Typical usage might be:

  <programlisting>
  # postgresql.conf
  shared_preload_libraries = 'pg_stat_statements'

  pg_stat_statements.max = 10000
  pg_stat_statements.track = all
  </programlisting>
diff --git a/doc/src/sgml/plperl.sgml b/doc/src/sgml/plperl.sgml
index b957757da64685450b5961a2ddc7c114c3dd3336..81b6de7adb67bd76c0a317e878833536f55913e8 100644
*** a/doc/src/sgml/plperl.sgml
--- b/doc/src/sgml/plperl.sgml
*************** CREATE TRIGGER test_valid_id_trig
*** 1219,1228 ****

    <para>
    This section lists configuration parameters that affect <application>PL/Perl</>.
-   To set any of these parameters before <application>PL/Perl</> has been loaded,
-   it is necessary to have added <quote><literal>plperl</></> to the
-   <xref linkend="guc-custom-variable-classes"> list in
-   <filename>postgresql.conf</filename>.
    </para>

    <variablelist>
--- 1219,1224 ----
diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
index 84fb012d1f3942914f12445807f9e6c0aa62eaa9..c14c34cd322488bc326b3e703d8060f9f3371599 100644
*** a/doc/src/sgml/plpgsql.sgml
--- b/doc/src/sgml/plpgsql.sgml
*************** BEGIN
*** 4007,4017 ****
      <literal>use_column</> (where <literal>error</> is the factory default).
      This parameter affects subsequent compilations
      of statements in <application>PL/pgSQL</> functions, but not statements
!     already compiled in the current session.  To set the parameter before
!     <application>PL/pgSQL</> has been loaded, it is necessary to have added
!     <quote><literal>plpgsql</></> to the <xref
!     linkend="guc-custom-variable-classes"> list in
!     <filename>postgresql.conf</filename>.  Because changing this setting
      can cause unexpected changes in the behavior of <application>PL/pgSQL</>
      functions, it can only be changed by a superuser.
     </para>
--- 4007,4014 ----
      <literal>use_column</> (where <literal>error</> is the factory default).
      This parameter affects subsequent compilations
      of statements in <application>PL/pgSQL</> functions, but not statements
!     already compiled in the current session.
!     Because changing this setting
      can cause unexpected changes in the behavior of <application>PL/pgSQL</>
      functions, it can only be changed by a superuser.
     </para>
diff --git a/src/backend/utils/misc/guc-file.l b/src/backend/utils/misc/guc-file.l
index a7cf0378dca25f23c3dfed987c088663769e2bf5..7728544c5452650187e242d06d7b6c3353ef99d5 100644
*** a/src/backend/utils/misc/guc-file.l
--- b/src/backend/utils/misc/guc-file.l
*************** ProcessConfigFile(GucContext context)
*** 111,118 ****
      ConfigVariable *item,
                     *head,
                     *tail;
-     char       *cvc = NULL;
-     struct config_string *cvc_struct;
      int            i;

      /*
--- 111,116 ----
*************** ProcessConfigFile(GucContext context)
*** 139,188 ****
      }

      /*
-      * We need the proposed new value of custom_variable_classes to check
-      * custom variables with.  ParseConfigFile ensured that if it's in
-      * the file, it's first in the list.  But first check to see if we
-      * have an active value from the command line, which should override
-      * the file in any case.  (Since there's no relevant env var, the
-      * only possible nondefault sources are the file and ARGV.)
-      */
-     cvc_struct = (struct config_string *)
-         find_option("custom_variable_classes", false, elevel);
-     Assert(cvc_struct);
-     if (cvc_struct->gen.reset_source > PGC_S_FILE)
-     {
-         cvc = guc_strdup(elevel, cvc_struct->reset_val);
-         if (cvc == NULL)
-         {
-             error = true;
-             goto cleanup_list;
-         }
-     }
-     else if (head != NULL &&
-              guc_name_compare(head->name, "custom_variable_classes") == 0)
-     {
-         /*
-          * Need to canonicalize the value by calling the check hook.
-          */
-         void   *extra = NULL;
-
-         cvc = guc_strdup(elevel, head->value);
-         if (cvc == NULL)
-         {
-             error = true;
-             goto cleanup_list;
-         }
-         if (!call_string_check_hook(cvc_struct, &cvc, &extra,
-                                     PGC_S_FILE, elevel))
-         {
-             error = true;
-             goto cleanup_list;
-         }
-         if (extra)
-             free(extra);
-     }
-
-     /*
       * Mark all extant GUC variables as not present in the config file.
       * We need this so that we can tell below which ones have been removed
       * from the file since we last processed it.
--- 137,142 ----
*************** ProcessConfigFile(GucContext context)
*** 199,237 ****
       * quasi-syntactic check on the validity of the config file.  It is
       * important that the postmaster and all backends agree on the results
       * of this phase, else we will have strange inconsistencies about which
!      * processes accept a config file update and which don't.  Hence, custom
!      * variable names can only be checked against custom_variable_classes,
!      * not against any loadable modules that might (or might not) be present.
!      * Likewise, we don't attempt to validate the options' values here.
       *
       * In addition, the GUC_IS_IN_FILE flag is set on each existing GUC
       * variable mentioned in the file.
       */
      for (item = head; item; item = item->next)
      {
-         char   *sep = strchr(item->name, GUC_QUALIFIER_SEPARATOR);
          struct config_generic *record;

!         if (sep)
!         {
!             /* Custom variable, so check against custom_variable_classes */
!             if (!is_custom_class(item->name, sep - item->name, cvc))
!             {
!                 ereport(elevel,
!                         (errcode(ERRCODE_UNDEFINED_OBJECT),
!                          errmsg("unrecognized configuration parameter \"%s\" in file \"%s\" line %u",
!                                 item->name,
!                                 item->filename, item->sourceline)));
!                 error = true;
!                 continue;
!             }
!         }
!
          record = find_option(item->name, false, elevel);

          if (record)
              record->status |= GUC_IS_IN_FILE;
!         else if (!sep)
          {
              /* Invalid non-custom variable, so complain */
              ereport(elevel,
--- 153,181 ----
       * quasi-syntactic check on the validity of the config file.  It is
       * important that the postmaster and all backends agree on the results
       * of this phase, else we will have strange inconsistencies about which
!      * processes accept a config file update and which don't.  Hence, unknown
!      * custom variable names have to be accepted without complaint.  For the
!      * same reason, we don't attempt to validate the options' values here.
       *
       * In addition, the GUC_IS_IN_FILE flag is set on each existing GUC
       * variable mentioned in the file.
       */
      for (item = head; item; item = item->next)
      {
          struct config_generic *record;

!         /*
!          * Try to find the variable; but do not create a custom placeholder
!          * if it's not there already.
!          */
          record = find_option(item->name, false, elevel);

          if (record)
+         {
+             /* Found, so mark it as present in file */
              record->status |= GUC_IS_IN_FILE;
!         }
!         else if (strchr(item->name, GUC_QUALIFIER_SEPARATOR) == NULL)
          {
              /* Invalid non-custom variable, so complain */
              ereport(elevel,
*************** ProcessConfigFile(GucContext context)
*** 382,389 ****

   cleanup_list:
      FreeConfigVariables(head);
-     if (cvc)
-         free(cvc);

      if (error)
      {
--- 326,331 ----
*************** ParseConfigFp(FILE *fp, const char *conf
*** 581,620 ****
              pfree(opt_name);
              pfree(opt_value);
          }
-         else if (guc_name_compare(opt_name, "custom_variable_classes") == 0)
-         {
-             /*
-              * This variable must be processed first as it controls
-              * the validity of other variables; so it goes at the head
-              * of the result list.  If we already found a value for it,
-              * replace with this one.
-              */
-             item = *head_p;
-             if (item != NULL &&
-                 guc_name_compare(item->name, "custom_variable_classes") == 0)
-             {
-                 /* replace existing head item */
-                 pfree(item->name);
-                 pfree(item->value);
-                 item->name = opt_name;
-                 item->value = opt_value;
-                 item->filename = pstrdup(config_file);
-                 item->sourceline = ConfigFileLineno-1;
-             }
-             else
-             {
-                 /* prepend to list */
-                 item = palloc(sizeof *item);
-                 item->name = opt_name;
-                 item->value = opt_value;
-                 item->filename = pstrdup(config_file);
-                 item->sourceline = ConfigFileLineno-1;
-                 item->next = *head_p;
-                 *head_p = item;
-                 if (*tail_p == NULL)
-                     *tail_p = item;
-             }
-         }
          else
          {
              /* ordinary variable, append to list */
--- 523,528 ----
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index c5b14522d5fb0eec8beb545164727f128d716cd0..2fd4867d253ca97f1c1d10a31686f450a289b48e 100644
*** a/src/backend/utils/misc/guc.c
--- b/src/backend/utils/misc/guc.c
*************** static void assign_syslog_ident(const ch
*** 178,184 ****
  static void assign_session_replication_role(int newval, void *extra);
  static bool check_temp_buffers(int *newval, void **extra, GucSource source);
  static bool check_phony_autocommit(bool *newval, void **extra, GucSource source);
- static bool check_custom_variable_classes(char **newval, void **extra, GucSource source);
  static bool check_debug_assertions(bool *newval, void **extra, GucSource source);
  static bool check_bonjour(bool *newval, void **extra, GucSource source);
  static bool check_ssl(bool *newval, void **extra, GucSource source);
--- 178,183 ----
*************** static char *log_timezone_string;
*** 467,473 ****
  static char *timezone_abbreviations_string;
  static char *XactIsoLevel_string;
  static char *session_authorization_string;
- static char *custom_variable_classes;
  static int    max_function_args;
  static int    max_index_keys;
  static int    max_identifier_length;
--- 466,471 ----
*************** static struct config_string ConfigureNam
*** 2886,2902 ****
      },

      {
-         {"custom_variable_classes", PGC_SIGHUP, CUSTOM_OPTIONS,
-             gettext_noop("Sets the list of known custom variable classes."),
-             NULL,
-             GUC_LIST_INPUT | GUC_LIST_QUOTE
-         },
-         &custom_variable_classes,
-         NULL,
-         check_custom_variable_classes, NULL, NULL
-     },
-
-     {
          {"data_directory", PGC_POSTMASTER, FILE_LOCATIONS,
              gettext_noop("Sets the server's data directory."),
              NULL,
--- 2884,2889 ----
*************** add_guc_variable(struct config_generic *
*** 3623,3630 ****
  }

  /*
!  * Create and add a placeholder variable. It's presumed to belong
!  * to a valid custom variable class at this point.
   */
  static struct config_generic *
  add_placeholder_variable(const char *name, int elevel)
--- 3610,3616 ----
  }

  /*
!  * Create and add a placeholder variable for a custom variable name.
   */
  static struct config_generic *
  add_placeholder_variable(const char *name, int elevel)
*************** add_placeholder_variable(const char *nam
*** 3670,3711 ****
  }

  /*
-  * Detect whether the portion of "name" before dotPos matches any custom
-  * variable class name listed in custom_var_classes.  The latter must be
-  * formatted the way that assign_custom_variable_classes does it, ie,
-  * no whitespace.  NULL is valid for custom_var_classes.
-  */
- static bool
- is_custom_class(const char *name, int dotPos, const char *custom_var_classes)
- {
-     bool        result = false;
-     const char *ccs = custom_var_classes;
-
-     if (ccs != NULL)
-     {
-         const char *start = ccs;
-
-         for (;; ++ccs)
-         {
-             char        c = *ccs;
-
-             if (c == '\0' || c == ',')
-             {
-                 if (dotPos == ccs - start && strncmp(start, name, dotPos) == 0)
-                 {
-                     result = true;
-                     break;
-                 }
-                 if (c == '\0')
-                     break;
-                 start = ccs + 1;
-             }
-         }
-     }
-     return result;
- }
-
- /*
   * Look up option NAME.  If it exists, return a pointer to its record,
   * else return NULL.  If create_placeholders is TRUE, we'll create a
   * placeholder record for a valid-looking custom variable name.
--- 3656,3661 ----
*************** find_option(const char *name, bool creat
*** 3745,3757 ****
      if (create_placeholders)
      {
          /*
!          * Check if the name is qualified, and if so, check if the qualifier
!          * matches any custom variable class.  If so, add a placeholder.
           */
!         const char *dot = strchr(name, GUC_QUALIFIER_SEPARATOR);
!
!         if (dot != NULL &&
!             is_custom_class(name, dot - name, custom_variable_classes))
              return add_placeholder_variable(name, elevel);
      }

--- 3695,3703 ----
      if (create_placeholders)
      {
          /*
!          * Check if the name is qualified, and if so, add a placeholder.
           */
!         if (strchr(name, GUC_QUALIFIER_SEPARATOR) != NULL)
              return add_placeholder_variable(name, elevel);
      }

*************** write_nondefault_variables(GucContext co
*** 7406,7412 ****
  {
      int            elevel;
      FILE       *fp;
-     struct config_generic *cvc_conf;
      int            i;

      Assert(context == PGC_POSTMASTER || context == PGC_SIGHUP);
--- 7352,7357 ----
*************** write_nondefault_variables(GucContext co
*** 7426,7445 ****
          return;
      }

-     /*
-      * custom_variable_classes must be written out first; otherwise we might
-      * reject custom variable values while reading the file.
-      */
-     cvc_conf = find_option("custom_variable_classes", false, ERROR);
-     if (cvc_conf)
-         write_one_nondefault_variable(fp, cvc_conf);
-
      for (i = 0; i < num_guc_variables; i++)
      {
!         struct config_generic *gconf = guc_variables[i];
!
!         if (gconf != cvc_conf)
!             write_one_nondefault_variable(fp, gconf);
      }

      if (FreeFile(fp))
--- 7371,7379 ----
          return;
      }

      for (i = 0; i < num_guc_variables; i++)
      {
!         write_one_nondefault_variable(fp, guc_variables[i]);
      }

      if (FreeFile(fp))
*************** validate_option_array_item(const char *n
*** 7886,7905 ****
       * permissions normally (ie, allow if variable is USERSET, or if it's
       * SUSET and user is superuser).
       *
!      * name is not known, but exists or can be created as a placeholder
!      * (implying it has a prefix listed in custom_variable_classes). We allow
!      * this case if you're a superuser, otherwise not.  Superusers are assumed
!      * to know what they're doing.  We can't allow it for other users, because
!      * when the placeholder is resolved it might turn out to be a SUSET
!      * variable; define_custom_variable assumes we checked that.
       *
       * name is not known and can't be created as a placeholder.  Throw error,
!      * unless skipIfNoPermissions is true, in which case return FALSE. (It's
!      * tempting to allow this case to superusers, if the name is qualified but
!      * not listed in custom_variable_classes.  That would ease restoring of
!      * dumps containing ALTER ROLE/DATABASE SET.  However, it's not clear that
!      * this usage justifies such a loss of error checking. You can always fix
!      * custom_variable_classes before you restore.)
       */
      gconf = find_option(name, true, WARNING);
      if (!gconf)
--- 7820,7834 ----
       * permissions normally (ie, allow if variable is USERSET, or if it's
       * SUSET and user is superuser).
       *
!      * name is not known, but exists or can be created as a placeholder (i.e.,
!      * it has a prefixed name).  We allow this case if you're a superuser,
!      * otherwise not.  Superusers are assumed to know what they're doing.
!      * We can't allow it for other users, because when the placeholder is
!      * resolved it might turn out to be a SUSET variable;
!      * define_custom_variable assumes we checked that.
       *
       * name is not known and can't be created as a placeholder.  Throw error,
!      * unless skipIfNoPermissions is true, in which case return FALSE.
       */
      gconf = find_option(name, true, WARNING);
      if (!gconf)
*************** validate_option_array_item(const char *n
*** 7909,7915 ****
              return false;
          ereport(ERROR,
                  (errcode(ERRCODE_UNDEFINED_OBJECT),
!                errmsg("unrecognized configuration parameter \"%s\"", name)));
      }

      if (gconf->flags & GUC_CUSTOM_PLACEHOLDER)
--- 7838,7845 ----
              return false;
          ereport(ERROR,
                  (errcode(ERRCODE_UNDEFINED_OBJECT),
!                  errmsg("unrecognized configuration parameter \"%s\"",
!                         name)));
      }

      if (gconf->flags & GUC_CUSTOM_PLACEHOLDER)
*************** check_phony_autocommit(bool *newval, voi
*** 8263,8331 ****
  }

  static bool
- check_custom_variable_classes(char **newval, void **extra, GucSource source)
- {
-     /*
-      * Check syntax. newval must be a comma separated list of identifiers.
-      * Whitespace is allowed but removed from the result.
-      */
-     bool        hasSpaceAfterToken = false;
-     const char *cp = *newval;
-     int            symLen = 0;
-     char        c;
-     StringInfoData buf;
-
-     /* Default NULL is OK */
-     if (cp == NULL)
-         return true;
-
-     initStringInfo(&buf);
-     while ((c = *cp++) != '\0')
-     {
-         if (isspace((unsigned char) c))
-         {
-             if (symLen > 0)
-                 hasSpaceAfterToken = true;
-             continue;
-         }
-
-         if (c == ',')
-         {
-             if (symLen > 0)        /* terminate identifier */
-             {
-                 appendStringInfoChar(&buf, ',');
-                 symLen = 0;
-             }
-             hasSpaceAfterToken = false;
-             continue;
-         }
-
-         if (hasSpaceAfterToken || !(isalnum((unsigned char) c) || c == '_'))
-         {
-             /*
-              * Syntax error due to token following space after token or
-              * non-identifier character
-              */
-             pfree(buf.data);
-             return false;
-         }
-         appendStringInfoChar(&buf, c);
-         symLen++;
-     }
-
-     /* Remove stray ',' at end */
-     if (symLen == 0 && buf.len > 0)
-         buf.data[--buf.len] = '\0';
-
-     /* GUC wants the result malloc'd */
-     free(*newval);
-     *newval = guc_strdup(LOG, buf.data);
-
-     pfree(buf.data);
-     return true;
- }
-
- static bool
  check_debug_assertions(bool *newval, void **extra, GucSource source)
  {
  #ifndef USE_ASSERT_CHECKING
--- 8193,8198 ----
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index a18f14ae253a432f4386682d2bcd55d61fd9dba3..5bb7e7117bc95c5cf1ba2bd312c707f4208999c5 100644
*** a/src/backend/utils/misc/postgresql.conf.sample
--- b/src/backend/utils/misc/postgresql.conf.sample
***************
*** 560,563 ****
  # CUSTOMIZED OPTIONS
  #------------------------------------------------------------------------------

! #custom_variable_classes = ''        # list of custom variable class names
--- 560,563 ----
  # CUSTOMIZED OPTIONS
  #------------------------------------------------------------------------------

! # Add settings for extensions here
diff --git a/src/pl/plperl/expected/plperl_init.out b/src/pl/plperl/expected/plperl_init.out
index 4a04dbd6f37d43fab912206c1dd23ea89d0be11d..e8a8e9bd83d6dff6c034d154965c86be55b1303d 100644
*** a/src/pl/plperl/expected/plperl_init.out
--- b/src/pl/plperl/expected/plperl_init.out
***************
*** 1,5 ****
  -- test plperl.on_plperl_init errors are fatal
! -- Avoid need for custom_variable_classes = 'plperl'
  LOAD 'plperl';
  SET SESSION plperl.on_plperl_init = ' system("/nonesuch") ';
  SHOW plperl.on_plperl_init;
--- 1,5 ----
  -- test plperl.on_plperl_init errors are fatal
! -- Must load plperl before we can set on_plperl_init
  LOAD 'plperl';
  SET SESSION plperl.on_plperl_init = ' system("/nonesuch") ';
  SHOW plperl.on_plperl_init;
diff --git a/src/pl/plperl/expected/plperl_shared.out b/src/pl/plperl/expected/plperl_shared.out
index 1a6bf5ee4d42eba0492cfca5c7dee8ad6af321b8..67478ab454b4176955ca36e236edfd704222972b 100644
*** a/src/pl/plperl/expected/plperl_shared.out
--- b/src/pl/plperl/expected/plperl_shared.out
***************
*** 1,6 ****
  -- test plperl.on_plperl_init via the shared hash
  -- (must be done before plperl is first used)
! -- Avoid need for custom_variable_classes = 'plperl'
  LOAD 'plperl';
  -- testing on_plperl_init gets run, and that it can alter %_SHARED
  SET plperl.on_plperl_init = '$_SHARED{on_init} = 42';
--- 1,6 ----
  -- test plperl.on_plperl_init via the shared hash
  -- (must be done before plperl is first used)
! -- Must load plperl before we can set on_plperl_init
  LOAD 'plperl';
  -- testing on_plperl_init gets run, and that it can alter %_SHARED
  SET plperl.on_plperl_init = '$_SHARED{on_init} = 42';
diff --git a/src/pl/plperl/expected/plperlu.out b/src/pl/plperl/expected/plperlu.out
index 25ac007b7a27b71558356c37adb2e2e9856b87b4..1ba07eed9dc835579e52bc50217075f64991dbbc 100644
*** a/src/pl/plperl/expected/plperlu.out
--- b/src/pl/plperl/expected/plperlu.out
***************
*** 1,6 ****
  -- Use ONLY plperlu tests here. For plperl/plerlu combined tests
  -- see plperl_plperlu.sql
! -- Avoid need for custom_variable_classes = 'plperl'
  LOAD 'plperl';
  -- Test plperl.on_plperlu_init gets run
  SET plperl.on_plperlu_init = '$_SHARED{init} = 42';
--- 1,6 ----
  -- Use ONLY plperlu tests here. For plperl/plerlu combined tests
  -- see plperl_plperlu.sql
! -- Must load plperl before we can set on_plperlu_init
  LOAD 'plperl';
  -- Test plperl.on_plperlu_init gets run
  SET plperl.on_plperlu_init = '$_SHARED{init} = 42';
diff --git a/src/pl/plperl/sql/plperl_init.sql b/src/pl/plperl/sql/plperl_init.sql
index f6a32b9bae4792ab449ef62cdb717384adca6bd8..51ac9192bdfc4f77c08a841f0106304ee34e9e56 100644
*** a/src/pl/plperl/sql/plperl_init.sql
--- b/src/pl/plperl/sql/plperl_init.sql
***************
*** 1,6 ****
  -- test plperl.on_plperl_init errors are fatal

! -- Avoid need for custom_variable_classes = 'plperl'
  LOAD 'plperl';

  SET SESSION plperl.on_plperl_init = ' system("/nonesuch") ';
--- 1,6 ----
  -- test plperl.on_plperl_init errors are fatal

! -- Must load plperl before we can set on_plperl_init
  LOAD 'plperl';

  SET SESSION plperl.on_plperl_init = ' system("/nonesuch") ';
diff --git a/src/pl/plperl/sql/plperl_shared.sql b/src/pl/plperl/sql/plperl_shared.sql
index d367d32ff08266f9b0f1f558e704e785fd3420ec..d2fa8cbf93e7b7f63c49984b1432a93de65089a7 100644
*** a/src/pl/plperl/sql/plperl_shared.sql
--- b/src/pl/plperl/sql/plperl_shared.sql
***************
*** 1,7 ****
  -- test plperl.on_plperl_init via the shared hash
  -- (must be done before plperl is first used)

! -- Avoid need for custom_variable_classes = 'plperl'
  LOAD 'plperl';

  -- testing on_plperl_init gets run, and that it can alter %_SHARED
--- 1,7 ----
  -- test plperl.on_plperl_init via the shared hash
  -- (must be done before plperl is first used)

! -- Must load plperl before we can set on_plperl_init
  LOAD 'plperl';

  -- testing on_plperl_init gets run, and that it can alter %_SHARED
diff --git a/src/pl/plperl/sql/plperlu.sql b/src/pl/plperl/sql/plperlu.sql
index 125691e5f7ba7bad01aae0a7c71a20c251e9c69e..831b8f44604e1829e058394fed9c67ccf6e75c7f 100644
*** a/src/pl/plperl/sql/plperlu.sql
--- b/src/pl/plperl/sql/plperlu.sql
***************
*** 1,7 ****
  -- Use ONLY plperlu tests here. For plperl/plerlu combined tests
  -- see plperl_plperlu.sql

! -- Avoid need for custom_variable_classes = 'plperl'
  LOAD 'plperl';

  -- Test plperl.on_plperlu_init gets run
--- 1,7 ----
  -- Use ONLY plperlu tests here. For plperl/plerlu combined tests
  -- see plperl_plperlu.sql

! -- Must load plperl before we can set on_plperlu_init
  LOAD 'plperl';

  -- Test plperl.on_plperlu_init gets run