Remember the source GucContext for each GUC parameter.

We used to just remember the GucSource, but saving GucContext too provides
a little more information --- notably, whether a SET was done by a
superuser or regular user.  This allows us to rip out the fairly dodgy code
that define_custom_variable used to use to try to infer the context to
re-install a pre-existing setting with.  In particular, it now works for
a superuser to SET a extension's SUSET custom variable before loading the
associated extension, because GUC can remember whether the SET was done as
a superuser or not.  The plperl regression tests contain an example where
this is useful.

diff --git a/src/backend/utils/misc/guc-file.l b/src/backend/utils/misc/guc-file.l
index 7728544c5452650187e242d06d7b6c3353ef99d5..b91da01e86804d484d165345065d874248f80794 100644 (file)
--- a/src/backend/utils/misc/guc-file.l
+++ b/src/backend/utils/misc/guc-file.l
@@ -296,11 +296,7 @@ ProcessConfigFile(GucContext context)
                                                                  GUC_ACTION_SET, true);
                if (scres > 0)
                {
-                       /* variable was updated, so remember the source location */
-                       set_config_sourcefile(item->name, item->filename,
-                                                                 item->sourceline);
-
-                       /* and log the change if appropriate */
+                       /* variable was updated, so log the change if appropriate */
                        if (pre_value)
                        {
                                const char *post_value = GetConfigOption(item->name, true, false);
@@ -315,7 +311,17 @@ ProcessConfigFile(GucContext context)
                }
                else if (scres == 0)
                        error = true;
-               /* else no error but variable was not changed, do nothing */
+               /* else no error but variable's active value was not changed */
+
+               /*
+                * We should update source location unless there was an error, since
+                * even if the active value didn't change, the reset value might have.
+                * (In the postmaster, there won't be a difference, but it does matter
+                * in backends.)
+                */
+               if (scres != 0)
+                       set_config_sourcefile(item->name, item->filename,
+                                                                 item->sourceline);
 
                if (pre_value)
                        pfree(pre_value);

diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 2fd4867d253ca97f1c1d10a31686f450a289b48e..84702aa46ed45c1ae86b13491fa3fc3527376bf6 100644 (file)
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -3861,8 +3861,10 @@ static void
 InitializeOneGUCOption(struct config_generic * gconf)
 {
        gconf->status = 0;
-       gconf->reset_source = PGC_S_DEFAULT;
        gconf->source = PGC_S_DEFAULT;
+       gconf->reset_source = PGC_S_DEFAULT;
+       gconf->scontext = PGC_INTERNAL;
+       gconf->reset_scontext = PGC_INTERNAL;
        gconf->stack = NULL;
        gconf->extra = NULL;
        gconf->sourcefile = NULL;
@@ -4213,6 +4215,7 @@ ResetAllOptions(void)
                }
 
                gconf->source = gconf->reset_source;
+               gconf->scontext = gconf->reset_scontext;
 
                if (gconf->flags & GUC_REPORT)
                        ReportGUCOption(gconf);
@@ -4254,6 +4257,7 @@ push_old_value(struct config_generic * gconf, GucAction action)
                                if (stack->state == GUC_SET)
                                {
                                        /* SET followed by SET LOCAL, remember SET's value */
+                                       stack->masked_scontext = gconf->scontext;
                                        set_stack_value(gconf, &stack->masked);
                                        stack->state = GUC_SET_LOCAL;
                                }
@@ -4291,6 +4295,7 @@ push_old_value(struct config_generic * gconf, GucAction action)
                        break;
        }
        stack->source = gconf->source;
+       stack->scontext = gconf->scontext;
        set_stack_value(gconf, &stack->prior);
 
        gconf->stack = stack;
@@ -4431,6 +4436,7 @@ AtEOXact_GUC(bool isCommit, int nestLevel)
                                                if (prev->state == GUC_SET)
                                                {
                                                        /* LOCAL migrates down */
+                                                       prev->masked_scontext = stack->scontext;
                                                        prev->masked = stack->prior;
                                                        prev->state = GUC_SET_LOCAL;
                                                }
@@ -4445,6 +4451,7 @@ AtEOXact_GUC(bool isCommit, int nestLevel)
                                                /* prior state at this level no longer wanted */
                                                discard_stack_value(gconf, &stack->prior);
                                                /* copy down the masked state */
+                                               prev->masked_scontext = stack->masked_scontext;
                                                if (prev->state == GUC_SET_LOCAL)
                                                        discard_stack_value(gconf, &prev->masked);
                                                prev->masked = stack->masked;
@@ -4460,16 +4467,19 @@ AtEOXact_GUC(bool isCommit, int nestLevel)
                                /* Perform appropriate restoration of the stacked value */
                                config_var_value newvalue;
                                GucSource       newsource;
+                               GucContext      newscontext;
 
                                if (restoreMasked)
                                {
                                        newvalue = stack->masked;
                                        newsource = PGC_S_SESSION;
+                                       newscontext = stack->masked_scontext;
                                }
                                else
                                {
                                        newvalue = stack->prior;
                                        newsource = stack->source;
+                                       newscontext = stack->scontext;
                                }
 
                                switch (gconf->vartype)
@@ -4581,7 +4591,9 @@ AtEOXact_GUC(bool isCommit, int nestLevel)
                                set_extra_field(gconf, &(stack->prior.extra), NULL);
                                set_extra_field(gconf, &(stack->masked.extra), NULL);
 
+                               /* And restore source information */
                                gconf->source = newsource;
+                               gconf->scontext = newscontext;
                        }
 
                        /* Finish popping the state stack */
@@ -5255,6 +5267,7 @@ set_config_option(const char *name, const char *value,
                                        newval = conf->reset_val;
                                        newextra = conf->reset_extra;
                                        source = conf->gen.reset_source;
+                                       context = conf->gen.reset_scontext;
                                }
 
                                if (prohibitValueChange)
@@ -5282,6 +5295,7 @@ set_config_option(const char *name, const char *value,
                                        set_extra_field(&conf->gen, &conf->gen.extra,
                                                                        newextra);
                                        conf->gen.source = source;
+                                       conf->gen.scontext = context;
                                }
                                if (makeDefault)
                                {
@@ -5293,6 +5307,7 @@ set_config_option(const char *name, const char *value,
                                                set_extra_field(&conf->gen, &conf->reset_extra,
                                                                                newextra);
                                                conf->gen.reset_source = source;
+                                               conf->gen.reset_scontext = context;
                                        }
                                        for (stack = conf->gen.stack; stack; stack = stack->prev)
                                        {
@@ -5302,6 +5317,7 @@ set_config_option(const char *name, const char *value,
                                                        set_extra_field(&conf->gen, &stack->prior.extra,
                                                                                        newextra);
                                                        stack->source = source;
+                                                       stack->scontext = context;
                                                }
                                        }
                                }
@@ -5355,6 +5371,7 @@ set_config_option(const char *name, const char *value,
                                        newval = conf->reset_val;
                                        newextra = conf->reset_extra;
                                        source = conf->gen.reset_source;
+                                       context = conf->gen.reset_scontext;
                                }
 
                                if (prohibitValueChange)
@@ -5382,6 +5399,7 @@ set_config_option(const char *name, const char *value,
                                        set_extra_field(&conf->gen, &conf->gen.extra,
                                                                        newextra);
                                        conf->gen.source = source;
+                                       conf->gen.scontext = context;
                                }
                                if (makeDefault)
                                {
@@ -5393,6 +5411,7 @@ set_config_option(const char *name, const char *value,
                                                set_extra_field(&conf->gen, &conf->reset_extra,
                                                                                newextra);
                                                conf->gen.reset_source = source;
+                                               conf->gen.reset_scontext = context;
                                        }
                                        for (stack = conf->gen.stack; stack; stack = stack->prev)
                                        {
@@ -5402,6 +5421,7 @@ set_config_option(const char *name, const char *value,
                                                        set_extra_field(&conf->gen, &stack->prior.extra,
                                                                                        newextra);
                                                        stack->source = source;
+                                                       stack->scontext = context;
                                                }
                                        }
                                }
@@ -5452,6 +5472,7 @@ set_config_option(const char *name, const char *value,
                                        newval = conf->reset_val;
                                        newextra = conf->reset_extra;
                                        source = conf->gen.reset_source;
+                                       context = conf->gen.reset_scontext;
                                }
 
                                if (prohibitValueChange)
@@ -5479,6 +5500,7 @@ set_config_option(const char *name, const char *value,
                                        set_extra_field(&conf->gen, &conf->gen.extra,
                                                                        newextra);
                                        conf->gen.source = source;
+                                       conf->gen.scontext = context;
                                }
                                if (makeDefault)
                                {
@@ -5490,6 +5512,7 @@ set_config_option(const char *name, const char *value,
                                                set_extra_field(&conf->gen, &conf->reset_extra,
                                                                                newextra);
                                                conf->gen.reset_source = source;
+                                               conf->gen.reset_scontext = context;
                                        }
                                        for (stack = conf->gen.stack; stack; stack = stack->prev)
                                        {
@@ -5499,6 +5522,7 @@ set_config_option(const char *name, const char *value,
                                                        set_extra_field(&conf->gen, &stack->prior.extra,
                                                                                        newextra);
                                                        stack->source = source;
+                                                       stack->scontext = context;
                                                }
                                        }
                                }
@@ -5567,6 +5591,7 @@ set_config_option(const char *name, const char *value,
                                        newval = conf->reset_val;
                                        newextra = conf->reset_extra;
                                        source = conf->gen.reset_source;
+                                       context = conf->gen.reset_scontext;
                                }
 
                                if (prohibitValueChange)
@@ -5596,6 +5621,7 @@ set_config_option(const char *name, const char *value,
                                        set_extra_field(&conf->gen, &conf->gen.extra,
                                                                        newextra);
                                        conf->gen.source = source;
+                                       conf->gen.scontext = context;
                                }
 
                                if (makeDefault)
@@ -5608,6 +5634,7 @@ set_config_option(const char *name, const char *value,
                                                set_extra_field(&conf->gen, &conf->reset_extra,
                                                                                newextra);
                                                conf->gen.reset_source = source;
+                                               conf->gen.reset_scontext = context;
                                        }
                                        for (stack = conf->gen.stack; stack; stack = stack->prev)
                                        {
@@ -5618,6 +5645,7 @@ set_config_option(const char *name, const char *value,
                                                        set_extra_field(&conf->gen, &stack->prior.extra,
                                                                                        newextra);
                                                        stack->source = source;
+                                                       stack->scontext = context;
                                                }
                                        }
                                }
@@ -5673,6 +5701,7 @@ set_config_option(const char *name, const char *value,
                                        newval = conf->reset_val;
                                        newextra = conf->reset_extra;
                                        source = conf->gen.reset_source;
+                                       context = conf->gen.reset_scontext;
                                }
 
                                if (prohibitValueChange)
@@ -5700,6 +5729,7 @@ set_config_option(const char *name, const char *value,
                                        set_extra_field(&conf->gen, &conf->gen.extra,
                                                                        newextra);
                                        conf->gen.source = source;
+                                       conf->gen.scontext = context;
                                }
                                if (makeDefault)
                                {
@@ -5711,6 +5741,7 @@ set_config_option(const char *name, const char *value,
                                                set_extra_field(&conf->gen, &conf->reset_extra,
                                                                                newextra);
                                                conf->gen.reset_source = source;
+                                               conf->gen.reset_scontext = context;
                                        }
                                        for (stack = conf->gen.stack; stack; stack = stack->prev)
                                        {
@@ -5720,6 +5751,7 @@ set_config_option(const char *name, const char *value,
                                                        set_extra_field(&conf->gen, &stack->prior.extra,
                                                                                        newextra);
                                                        stack->source = source;
+                                                       stack->scontext = context;
                                                }
                                        }
                                }
@@ -6252,7 +6284,6 @@ define_custom_variable(struct config_generic * variable)
        const char **nameAddr = &name;
        const char *value;
        struct config_string *pHolder;
-       GucContext      phcontext;
        struct config_generic **res;
 
        /*
@@ -6298,56 +6329,6 @@ define_custom_variable(struct config_generic * variable)
         */
        *res = variable;
 
-       /*
-        * Infer context for assignment based on source of existing value. We
-        * can't tell this with exact accuracy, but we can at least do something
-        * reasonable in typical cases.
-        */
-       switch (pHolder->gen.source)
-       {
-               case PGC_S_DEFAULT:
-               case PGC_S_DYNAMIC_DEFAULT:
-               case PGC_S_ENV_VAR:
-               case PGC_S_FILE:
-               case PGC_S_ARGV:
-
-                       /*
-                        * If we got past the check in init_custom_variable, we can safely
-                        * assume that any existing value for a PGC_POSTMASTER variable
-                        * was set in postmaster context.
-                        */
-                       if (variable->context == PGC_POSTMASTER)
-                               phcontext = PGC_POSTMASTER;
-                       else
-                               phcontext = PGC_SIGHUP;
-                       break;
-
-               case PGC_S_DATABASE:
-               case PGC_S_USER:
-               case PGC_S_DATABASE_USER:
-
-                       /*
-                        * The existing value came from an ALTER ROLE/DATABASE SET
-                        * command. We can assume that at the time the command was issued,
-                        * we checked that the issuing user was superuser if the variable
-                        * requires superuser privileges to set.  So it's safe to use
-                        * SUSET context here.
-                        */
-                       phcontext = PGC_SUSET;
-                       break;
-
-               case PGC_S_CLIENT:
-               case PGC_S_SESSION:
-               default:
-
-                       /*
-                        * We must assume that the value came from an untrusted user, even
-                        * if the current_user is a superuser.
-                        */
-                       phcontext = PGC_USERSET;
-                       break;
-       }
-
        /*
         * Assign the string value stored in the placeholder to the real variable.
         *
@@ -6360,8 +6341,8 @@ define_custom_variable(struct config_generic * variable)
        if (value)
        {
                if (set_config_option(name, value,
-                                                         phcontext, pHolder->gen.source,
-                                                         GUC_ACTION_SET, true) > 0)
+                                                         pHolder->gen.scontext, pHolder->gen.source,
+                                                         GUC_ACTION_SET, true) != 0)
                {
                        /* Also copy over any saved source-location information */
                        if (pHolder->gen.sourcefile)
@@ -7284,6 +7265,7 @@ _ShowOption(struct config_generic * record, bool use_units)
  *             variable name, string, null terminated
  *             variable value, string, null terminated
  *             variable source, integer
+ *             variable scontext, integer
  */
 static void
 write_one_nondefault_variable(FILE *fp, struct config_generic * gconf)
@@ -7319,8 +7301,7 @@ write_one_nondefault_variable(FILE *fp, struct config_generic * gconf)
                        {
                                struct config_real *conf = (struct config_real *) gconf;
 
-                               /* Could lose precision here? */
-                               fprintf(fp, "%f", *conf->variable);
+                               fprintf(fp, "%.17g", *conf->variable);
                        }
                        break;
 
@@ -7344,7 +7325,8 @@ write_one_nondefault_variable(FILE *fp, struct config_generic * gconf)
 
        fputc(0, fp);
 
-       fwrite(&gconf->source, sizeof(gconf->source), 1, fp);
+       fwrite(&gconf->source, 1, sizeof(gconf->source), fp);
+       fwrite(&gconf->scontext, 1, sizeof(gconf->scontext), fp);
 }
 
 void
@@ -7436,7 +7418,8 @@ read_nondefault_variables(void)
        FILE       *fp;
        char       *varname,
                           *varvalue;
-       int                     varsource;
+       GucSource       varsource;
+       GucContext      varscontext;
 
        /*
         * Open file
@@ -7464,11 +7447,14 @@ read_nondefault_variables(void)
                        elog(FATAL, "failed to locate variable %s in exec config params file", varname);
                if ((varvalue = read_string_with_null(fp)) == NULL)
                        elog(FATAL, "invalid format of exec config params file");
-               if (fread(&varsource, sizeof(varsource), 1, fp) == 0)
+               if (fread(&varsource, 1, sizeof(varsource), fp) != sizeof(varsource))
+                       elog(FATAL, "invalid format of exec config params file");
+               if (fread(&varscontext, 1, sizeof(varscontext), fp) != sizeof(varscontext))
                        elog(FATAL, "invalid format of exec config params file");
 
-               (void) set_config_option(varname, varvalue, record->context,
-                                                                varsource, GUC_ACTION_SET, true);
+               (void) set_config_option(varname, varvalue,
+                                                                varscontext, varsource,
+                                                                GUC_ACTION_SET, true);
                free(varname);
                free(varvalue);
        }

diff --git a/src/include/utils/guc_tables.h b/src/include/utils/guc_tables.h
index 84e9fb54d2fe87817988a9e4261f841678e0c2eb..21342f59acd9927518c011db9b0a8d1f94881fe9 100644 (file)
--- a/src/include/utils/guc_tables.h
+++ b/src/include/utils/guc_tables.h
@@ -118,9 +118,11 @@ typedef struct guc_stack
        int                     nest_level;             /* nesting depth at which we made entry */
        GucStackState state;            /* see enum above */
        GucSource       source;                 /* source of the prior value */
+       /* masked value's source must be PGC_S_SESSION, so no need to store it */
+       GucContext      scontext;               /* context that set the prior value */
+       GucContext      masked_scontext; /* context that set the masked value */
        config_var_value prior;         /* previous value of variable */
        config_var_value masked;        /* SET value in a GUC_SET_LOCAL entry */
-       /* masked value's source must be PGC_S_SESSION, so no need to store it */
 } GucStack;
 
 /*
@@ -143,21 +145,21 @@ struct config_generic
        enum config_group group;        /* to help organize variables by function */
        const char *short_desc;         /* short desc. of this variable's purpose */
        const char *long_desc;          /* long desc. of this variable's purpose */
-       int                     flags;                  /* flag bits, see below */
+       int                     flags;                  /* flag bits, see guc.h */
        /* variable fields, initialized at runtime: */
        enum config_type vartype;       /* type of variable (set only at startup) */
        int                     status;                 /* status bits, see below */
-       GucSource       reset_source;   /* source of the reset_value */
        GucSource       source;                 /* source of the current actual value */
+       GucSource       reset_source;   /* source of the reset_value */
+       GucContext      scontext;               /* context that set the current value */
+       GucContext      reset_scontext; /* context that set the reset value */
        GucStack   *stack;                      /* stacked prior values */
        void       *extra;                      /* "extra" pointer for current actual value */
        char       *sourcefile;         /* file current setting is from (NULL if not
-                                                                * file) */
+                                                                * set in config file) */
        int                     sourceline;             /* line in source file */
 };
 
-/* bit values in flags field are defined in guc.h */
-
 /* bit values in status field */
 #define GUC_IS_IN_FILE         0x0001          /* found it in config file */
 /*

diff --git a/src/pl/plperl/expected/plperl_init.out b/src/pl/plperl/expected/plperl_init.out
index e8a8e9bd83d6dff6c034d154965c86be55b1303d..a21ea0b6214b891bad9a27999a0454a90836a456 100644 (file)
--- a/src/pl/plperl/expected/plperl_init.out
+++ b/src/pl/plperl/expected/plperl_init.out
@@ -1,5 +1,5 @@
 -- test plperl.on_plperl_init errors are fatal
--- Must load plperl before we can set on_plperl_init
+-- This test tests setting on_plperl_init after loading plperl
 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 67478ab454b4176955ca36e236edfd704222972b..464e22090c328b9cd92552f51ba5da21734eaef8 100644 (file)
--- a/src/pl/plperl/expected/plperl_shared.out
+++ b/src/pl/plperl/expected/plperl_shared.out
@@ -1,7 +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';
+-- This test tests setting on_plperl_init before loading plperl
 -- testing on_plperl_init gets run, and that it can alter %_SHARED
 SET plperl.on_plperl_init = '$_SHARED{on_init} = 42';
 -- test the shared hash

diff --git a/src/pl/plperl/expected/plperlu.out b/src/pl/plperl/expected/plperlu.out
index 1ba07eed9dc835579e52bc50217075f64991dbbc..3daf4ced86fbd2d29fa1ecc3e1c84cde65abf2e2 100644 (file)
--- a/src/pl/plperl/expected/plperlu.out
+++ b/src/pl/plperl/expected/plperlu.out
@@ -1,6 +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
+-- This test tests setting on_plperlu_init after loading plperl
 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 51ac9192bdfc4f77c08a841f0106304ee34e9e56..d60268d033ecf8cc6ef636ed933a5d6983fd1bab 100644 (file)
--- a/src/pl/plperl/sql/plperl_init.sql
+++ b/src/pl/plperl/sql/plperl_init.sql
@@ -1,6 +1,6 @@
 -- test plperl.on_plperl_init errors are fatal
 
--- Must load plperl before we can set on_plperl_init
+-- This test tests setting on_plperl_init after loading plperl
 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 d2fa8cbf93e7b7f63c49984b1432a93de65089a7..b60e114fafc7619b981ec9138d81eea0024c8170 100644 (file)
--- a/src/pl/plperl/sql/plperl_shared.sql
+++ b/src/pl/plperl/sql/plperl_shared.sql
@@ -1,8 +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';
+-- This test tests setting on_plperl_init before loading 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/sql/plperlu.sql b/src/pl/plperl/sql/plperlu.sql
index 831b8f44604e1829e058394fed9c67ccf6e75c7f..be43df5d90a6e1cc275609cda6035d937781b1f6 100644 (file)
--- a/src/pl/plperl/sql/plperlu.sql
+++ b/src/pl/plperl/sql/plperlu.sql
@@ -1,7 +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
+-- This test tests setting on_plperlu_init after loading plperl
 LOAD 'plperl';
 
 -- Test plperl.on_plperlu_init gets run