On Tue, Jul 15, 2014 at 10:45 PM, Magnus Hagander <mag...@hagander.net> wrote:
> On Wed, Jul 2, 2014 at 10:52 PM, Tom Lane <t...@sss.pgh.pa.us> wrote:
>> I wrote:
>>> In short, maybe we ought to invent a new category PGC_SU_BACKEND (not
>>> wedded to this spelling), which is to PGC_BACKEND as PGC_SUSET is to
>>> PGC_USERSET, ie same when-it-can-be-changed behavior but only superusers
>>> are allowed to change it.  I don't have any objection to making these two
>>> settings only adjustable by superusers --- I just don't want to give up
>>> the existing timing restrictions for them.
>>
>> Another idea would be to get rid of PGC_SUSET as a separate category, and
>> instead have a superuser-only bit in the GUC flags, which would apply to
>> all categories.  This would be a bit more orthogonal, though likely a
>> much more invasive change.
>
> That could become interesting in the futuren ow that we have ALTER
> SYSTEM SET. It could allow a non-superuser to make persistent
> configuration changes. Now, I'm not sure we actually *want* that
> though... But having it as a separate bit would make it possible for
> ALTER SYSTEM SET to say that for example regular users would be able
> to change work_mem persistently. But if we want to go down that route,
> we might need a more fine grained permissions model than just
> superuser vs non-superuser...
>
> I think going with the PGC_SU_BACKEND is the right choice at this
> time, until we have an actual usecase for the other :)

Yep, the attached patch introduces PGC_SU_BACKEND and
changes the contexts of log_connections and log_disconnections
to PGC_SU_BACKEND. Review?

Regards,

-- 
Fujii Masao
*** a/src/backend/tcop/postgres.c
--- b/src/backend/tcop/postgres.c
***************
*** 3258,3264 **** get_stats_option_name(const char *arg)
   * argv[0] is ignored in either case (it's assumed to be the program name).
   *
   * ctx is PGC_POSTMASTER for secure options, PGC_BACKEND for insecure options
!  * coming from the client, or PGC_SUSET for insecure options coming from
   * a superuser client.
   *
   * If a database name is present in the command line arguments, it's
--- 3258,3264 ----
   * argv[0] is ignored in either case (it's assumed to be the program name).
   *
   * ctx is PGC_POSTMASTER for secure options, PGC_BACKEND for insecure options
!  * coming from the client, or PGC_SU_BACKEND for insecure options coming from
   * a superuser client.
   *
   * If a database name is present in the command line arguments, it's
*** a/src/backend/utils/init/postinit.c
--- b/src/backend/utils/init/postinit.c
***************
*** 957,963 **** process_startup_options(Port *port, bool am_superuser)
  	GucContext	gucctx;
  	ListCell   *gucopts;
  
! 	gucctx = am_superuser ? PGC_SUSET : PGC_BACKEND;
  
  	/*
  	 * First process any command-line switches that were included in the
--- 957,963 ----
  	GucContext	gucctx;
  	ListCell   *gucopts;
  
! 	gucctx = am_superuser ? PGC_SU_BACKEND : PGC_BACKEND;
  
  	/*
  	 * First process any command-line switches that were included in the
*** a/src/backend/utils/misc/guc.c
--- b/src/backend/utils/misc/guc.c
***************
*** 512,517 **** const char *const GucContext_Names[] =
--- 512,518 ----
  	 /* PGC_INTERNAL */ "internal",
  	 /* PGC_POSTMASTER */ "postmaster",
  	 /* PGC_SIGHUP */ "sighup",
+ 	 /* PGC_SU_BACKEND */ "superuser-backend",
  	 /* PGC_BACKEND */ "backend",
  	 /* PGC_SUSET */ "superuser",
  	 /* PGC_USERSET */ "user"
***************
*** 910,916 **** static struct config_bool ConfigureNamesBool[] =
  		NULL, NULL, NULL
  	},
  	{
! 		{"log_connections", PGC_BACKEND, LOGGING_WHAT,
  			gettext_noop("Logs each successful connection."),
  			NULL
  		},
--- 911,917 ----
  		NULL, NULL, NULL
  	},
  	{
! 		{"log_connections", PGC_SU_BACKEND, LOGGING_WHAT,
  			gettext_noop("Logs each successful connection."),
  			NULL
  		},
***************
*** 919,925 **** static struct config_bool ConfigureNamesBool[] =
  		NULL, NULL, NULL
  	},
  	{
! 		{"log_disconnections", PGC_BACKEND, LOGGING_WHAT,
  			gettext_noop("Logs end of a session, including duration."),
  			NULL
  		},
--- 920,926 ----
  		NULL, NULL, NULL
  	},
  	{
! 		{"log_disconnections", PGC_SU_BACKEND, LOGGING_WHAT,
  			gettext_noop("Logs end of a session, including duration."),
  			NULL
  		},
***************
*** 5681,5696 **** set_config_option(const char *name, const char *value,
  			 * signals to individual backends only.
  			 */
  			break;
  		case PGC_BACKEND:
  			if (context == PGC_SIGHUP)
  			{
  				/*
! 				 * If a PGC_BACKEND parameter is changed in the config file,
! 				 * we want to accept the new value in the postmaster (whence
! 				 * it will propagate to subsequently-started backends), but
! 				 * ignore it in existing backends.  This is a tad klugy, but
! 				 * necessary because we don't re-read the config file during
! 				 * backend start.
  				 *
  				 * In EXEC_BACKEND builds, this works differently: we load all
  				 * nondefault settings from the CONFIG_EXEC_PARAMS file during
--- 5682,5706 ----
  			 * signals to individual backends only.
  			 */
  			break;
+ 		case PGC_SU_BACKEND:
+ 			if (context == PGC_BACKEND)
+ 			{
+ 				ereport(elevel,
+ 						(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ 						 errmsg("permission denied to set parameter \"%s\"",
+ 								name)));
+ 				return 0;
+ 			}
  		case PGC_BACKEND:
  			if (context == PGC_SIGHUP)
  			{
  				/*
! 				 * If a PGC_SU_BACKEND or PGC_BACKEND parameter is changed in
! 				 * the config file, we want to accept the new value in the
! 				 * postmaster (whence it will propagate to subsequently-started
! 				 * backends), but ignore it in existing backends.  This is a tad
! 				 * klugy, but necessary because we don't re-read the config file
! 				 * during backend start.
  				 *
  				 * In EXEC_BACKEND builds, this works differently: we load all
  				 * nondefault settings from the CONFIG_EXEC_PARAMS file during
***************
*** 5709,5716 **** set_config_option(const char *name, const char *value,
  					return -1;
  #endif
  			}
! 			else if (context != PGC_POSTMASTER && context != PGC_BACKEND &&
! 					 source != PGC_S_CLIENT)
  			{
  				ereport(elevel,
  						(errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
--- 5719,5726 ----
  					return -1;
  #endif
  			}
! 			else if (context != PGC_POSTMASTER && context != PGC_SU_BACKEND &&
! 					 context != PGC_SU_BACKEND && source != PGC_S_CLIENT)
  			{
  				ereport(elevel,
  						(errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
***************
*** 8353,8360 **** read_nondefault_variables(void)
  	GucContext	varscontext;
  
  	/*
! 	 * Assert that PGC_BACKEND case in set_config_option() will do the right
! 	 * thing.
  	 */
  	Assert(IsInitProcessingMode());
  
--- 8363,8370 ----
  	GucContext	varscontext;
  
  	/*
! 	 * Assert that PGC_SU_BACKEND and PGC_BACKEND case in set_config_option()
! 	 * will do the right thing.
  	 */
  	Assert(IsInitProcessingMode());
  
*** a/src/include/utils/guc.h
--- b/src/include/utils/guc.h
***************
*** 36,47 ****
   * certain point in their main loop. It's safer to wait than to read a
   * file asynchronously.)
   *
!  * BACKEND options can only be set at postmaster startup, from the
   * configuration file, or by client request in the connection startup
!  * packet (e.g., from libpq's PGOPTIONS variable).  Furthermore, an
!  * already-started backend will ignore changes to such an option in the
!  * configuration file.  The idea is that these options are fixed for a
!  * given backend once it's started, but they can vary across backends.
   *
   * SUSET options can be set at postmaster startup, with the SIGHUP
   * mechanism, or from SQL if you're a superuser.
--- 36,52 ----
   * certain point in their main loop. It's safer to wait than to read a
   * file asynchronously.)
   *
!  * SU_BACKEND options can only be set at postmaster startup, from the
   * configuration file, or by client request in the connection startup
!  * packet (e.g., from libpq's PGOPTIONS variable) if you're a superuser.
!  * Furthermore, an already-started backend will ignore changes to
!  * such an option in the configuration file.  The idea is that these options
!  * are fixed for a given backend once it's started, but they can vary
!  * across backends.
!  *
!  * BACKEND options are the same as SU_BACKEND ones, but they can
!  * be set by client request in the connection startup packet even when
!  * you're not a superuser.
   *
   * SUSET options can be set at postmaster startup, with the SIGHUP
   * mechanism, or from SQL if you're a superuser.
***************
*** 53,58 **** typedef enum
--- 58,64 ----
  	PGC_INTERNAL,
  	PGC_POSTMASTER,
  	PGC_SIGHUP,
+ 	PGC_SU_BACKEND,
  	PGC_BACKEND,
  	PGC_SUSET,
  	PGC_USERSET
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to