Nathan Bossart <nathandboss...@gmail.com> writes:
> On Fri, Aug 09, 2024 at 02:43:59PM -0400, Tom Lane wrote:
>> The simplest fix would be to hack this test to allow the action anyway
>> when context == PGC_INTERNAL, excusing that as "assume the caller
>> knows what it's doing". That feels pretty grotty though. Perhaps
>> a cleaner way would be to move this check to some higher code level,
>> but I'm not sure where would be a good place.
> From a couple of quick tests, it looks like setting
> "current_role_is_superuser" directly works.
Yeah, I had been thinking along the same lines. Here's a draft
patch. (Still needs some attention to nearby comments, and I can't
avoid the impression that the miscinit.c code in this area could
use refactoring.)
A problem with this is that it couldn't readily be back-patched
further than v14, since we didn't have ReportChangedGUCOptions
before that. Maybe that doesn't matter; given the lack of
previous complaints, maybe we only need to fix this in HEAD.
regards, tom lane
diff --git a/src/backend/commands/variable.c b/src/backend/commands/variable.c
index 6202c5ebe4..7b98b03c10 100644
--- a/src/backend/commands/variable.c
+++ b/src/backend/commands/variable.c
@@ -1026,6 +1026,19 @@ show_role(void)
return role_string ? role_string : "none";
}
+const char *
+show_is_superuser(void)
+{
+ /*
+ * Report the real value of current_role_is_superuser, not the
+ * possibly-stale value in the GUC mechanism. The reason we do things
+ * this way is to avoid the overhead and extra failure modes that'd be
+ * involved in using SetConfigOption to set is_superuser every time we
+ * update the session_authorization or role GUCs.
+ */
+ return current_role_is_superuser ? "on" : "off";
+}
+
/*
* PATH VARIABLES
diff --git a/src/backend/utils/init/miscinit.c b/src/backend/utils/init/miscinit.c
index 537d92c0cf..4052b16ecb 100644
--- a/src/backend/utils/init/miscinit.c
+++ b/src/backend/utils/init/miscinit.c
@@ -822,9 +822,7 @@ InitializeSessionUserId(const char *rolename, Oid roleid, bool bypass_login_chec
/* Record username and superuser status as GUC settings too */
SetConfigOption("session_authorization", rname,
PGC_BACKEND, PGC_S_OVERRIDE);
- SetConfigOption("is_superuser",
- is_superuser ? "on" : "off",
- PGC_INTERNAL, PGC_S_DYNAMIC_DEFAULT);
+ current_role_is_superuser = is_superuser;
ReleaseSysCache(roleTup);
}
@@ -854,8 +852,7 @@ InitializeSessionUserIdStandalone(void)
* Since we don't, C code will get NULL, and current_setting() will get an
* empty string.
*/
- SetConfigOption("is_superuser", "on",
- PGC_INTERNAL, PGC_S_DYNAMIC_DEFAULT);
+ current_role_is_superuser = true;
}
/*
@@ -909,9 +906,7 @@ SetSessionAuthorization(Oid userid, bool is_superuser)
{
SetSessionUserId(userid, is_superuser);
- SetConfigOption("is_superuser",
- is_superuser ? "on" : "off",
- PGC_INTERNAL, PGC_S_DYNAMIC_DEFAULT);
+ current_role_is_superuser = is_superuser;
}
/*
@@ -966,9 +961,7 @@ SetCurrentRoleId(Oid roleid, bool is_superuser)
SetOuterUserId(roleid);
- SetConfigOption("is_superuser",
- is_superuser ? "on" : "off",
- PGC_INTERNAL, PGC_S_DYNAMIC_DEFAULT);
+ current_role_is_superuser = is_superuser;
}
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 0c593b81b4..f99975ed61 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -2610,6 +2610,15 @@ ReportChangedGUCOptions(void)
SetConfigOption("in_hot_standby", "false",
PGC_INTERNAL, PGC_S_OVERRIDE);
+ /*
+ * Also, check to see if we need to update the is_superuser GUC. This is
+ * to queue a guc_report_list entry if needed.
+ */
+ if (current_role_is_superuser != current_role_is_superuser_guc)
+ SetConfigOption("is_superuser",
+ current_role_is_superuser ? "on" : "off",
+ PGC_INTERNAL, PGC_S_DYNAMIC_DEFAULT);
+
/* Transmit new values of interesting variables */
slist_foreach_modify(iter, &guc_report_list)
{
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index c0a52cdcc3..cf09c59e40 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -516,7 +516,8 @@ bool check_function_bodies = true;
*/
static bool default_with_oids = false;
-bool current_role_is_superuser;
+bool current_role_is_superuser; /* this is the "real" status */
+bool current_role_is_superuser_guc; /* but the GUC table points here */
int log_min_error_statement = ERROR;
int log_min_messages = WARNING;
@@ -1017,9 +1018,9 @@ struct config_bool ConfigureNamesBool[] =
NULL,
GUC_REPORT | GUC_NO_SHOW_ALL | GUC_NO_RESET_ALL | GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE
},
- ¤t_role_is_superuser,
+ ¤t_role_is_superuser_guc,
false,
- NULL, NULL, NULL
+ NULL, NULL, show_is_superuser
},
{
/*
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index 4129ea37ee..efdb4fc815 100644
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -260,6 +260,7 @@ extern PGDLLIMPORT char *event_source;
extern PGDLLIMPORT bool check_function_bodies;
extern PGDLLIMPORT bool current_role_is_superuser;
+extern PGDLLIMPORT bool current_role_is_superuser_guc; /* don't use this */
extern PGDLLIMPORT bool AllowAlterSystem;
extern PGDLLIMPORT bool log_duration;
diff --git a/src/include/utils/guc_hooks.h b/src/include/utils/guc_hooks.h
index 5813dba0a2..d10eb44a2c 100644
--- a/src/include/utils/guc_hooks.h
+++ b/src/include/utils/guc_hooks.h
@@ -121,6 +121,7 @@ extern void assign_recovery_target_xid(const char *newval, void *extra);
extern bool check_role(char **newval, void **extra, GucSource source);
extern void assign_role(const char *newval, void *extra);
extern const char *show_role(void);
+extern const char *show_is_superuser(void);
extern bool check_restrict_nonsystem_relation_kind(char **newval, void **extra,
GucSource source);
extern void assign_restrict_nonsystem_relation_kind(const char *newval, void *extra);
