refactor: use C APIs on alter_owner instead of SPI

The code is shorter and there's less room for error.

Author: steve-chavez

src/pg_prelude.h

@@ -10,11 +10,15 @@
 #include <catalog/pg_authid.h>
 #include <catalog/pg_proc.h>
 #include <commands/defrem.h>
+#include <commands/publicationcmds.h>
+#include <commands/defrem.h>
+#include <commands/event_trigger.h>
 #include <executor/spi.h>
 #include <fmgr.h>
 #include <miscadmin.h>
 #include <nodes/makefuncs.h>
 #include <nodes/pg_list.h>
+#include <nodes/value.h>
 #include <parser/parse_func.h>
 #include <tcop/utility.h>
 #include <tsearch/ts_locale.h>

src/supautils.c

@@ -553,10 +553,9 @@ static void supautils_hook(PROCESS_UTILITY_PARAMS) {
       run_process_utility_hook(prev_hook);
 
       CreateFdwStmt *stmt = (CreateFdwStmt *)utility_stmt;
-      const char *current_role_name = GetUserNameFromId(current_user_id, false);
 
       // Change FDW owner to the current role (which is a privileged role)
-      alter_owner(stmt->fdwname, current_role_name, ALT_FDW);
+      alter_owner(stmt->fdwname, current_user_id, ALT_FDW);
 
       if (!already_switched_to_superuser) {
           switch_to_original_role();
@@ -584,10 +583,9 @@ static void supautils_hook(PROCESS_UTILITY_PARAMS) {
       run_process_utility_hook(prev_hook);
 
       CreatePublicationStmt *stmt = (CreatePublicationStmt *)utility_stmt;
-      const char *current_role_name = GetUserNameFromId(current_user_id, false);
 
       // Change publication owner to the current role (which is a privileged role)
-      alter_owner(stmt->pubname, current_role_name, ALT_PUB);
+      alter_owner(stmt->pubname, current_user_id, ALT_PUB);
 
       if (!already_switched_to_superuser) {
           switch_to_original_role();
@@ -846,7 +844,6 @@ static void supautils_hook(PROCESS_UTILITY_PARAMS) {
           const Oid current_user_id = GetUserId();
 
           CreateEventTrigStmt *stmt = (CreateEventTrigStmt *)utility_stmt;
-          const char *current_role_name = GetUserNameFromId(current_user_id, false);
 
           bool current_user_is_super = superuser_arg(current_user_id);
           func_attrs fattrs = get_function_attrs((func_search){FO_SEARCH_NAME, {stmt->funcname}});
@@ -855,14 +852,14 @@ static void supautils_hook(PROCESS_UTILITY_PARAMS) {
           if(!current_user_is_super && function_is_owned_by_super){
             ereport(ERROR, (
               errmsg("Non-superuser owned event trigger must execute a non-superuser owned function")
-            , errdetail("The current user \"%s\" is not a superuser and the function \"%s\" is owned by a superuser", current_role_name, NameListToString(stmt->funcname))
+            , errdetail("The current user \"%s\" is not a superuser and the function \"%s\" is owned by a superuser", GetUserNameFromId(current_user_id, false), NameListToString(stmt->funcname))
             ));
           }
 
           if(current_user_is_super && !function_is_owned_by_super){
             ereport(ERROR, (
               errmsg("Superuser owned event trigger must execute a superuser owned function")
-            , errdetail("The current user \"%s\" is a superuser and the function \"%s\" is owned by a non-superuser", current_role_name, NameListToString(stmt->funcname))
+            , errdetail("The current user \"%s\" is a superuser and the function \"%s\" is owned by a non-superuser", GetUserNameFromId(current_user_id, false), NameListToString(stmt->funcname))
             ));
           }
 
@@ -872,7 +869,7 @@ static void supautils_hook(PROCESS_UTILITY_PARAMS) {
 
           if (!current_user_is_super)
             // Change event trigger owner to the current role (which is a privileged role)
-            alter_owner(stmt->trigname, current_role_name, ALT_EVTRIG);
+            alter_owner(stmt->trigname, current_user_id, ALT_EVTRIG);
 
           if (!already_switched_to_superuser) {
               switch_to_original_role();

src/utils.c

@@ -81,67 +81,62 @@ bool remove_ending_wildcard(char *elem) {
     return wildcard_removed;
 }
 
+static void alter_role_super(const char* rolename, bool make_super){
+  RoleSpec *rolespec   = makeNode(RoleSpec);
+  rolespec->roletype   = ROLESPEC_CSTRING;
+  rolespec->rolename   = pstrdup(rolename);
+  rolespec->location   = -1;
+
+  AlterRoleStmt *alter_stmt = makeNode(AlterRoleStmt);
+  alter_stmt->role          = rolespec;
+
+  alter_stmt->options = list_make1(
+    makeDefElem("superuser", (Node *) makeInteger(make_super), -1) // using makeInteger because makeBoolean is not available on pg <= 14
+  );
+
+#if PG15_GTE
+  AlterRole(NULL, alter_stmt);
+#else
+  AlterRole(alter_stmt);
+#endif
+
+  CommandCounterIncrement();
+}
+
 // Changes the OWNER of a database object.
 // Some objects (e.g. foreign data wrappers) can only be owned by superusers, so this switches to superuser accordingly and then goes backs to non-super.
-void alter_owner(const char *obj_name, const char *role_name, altered_obj_type obj_type){
+void alter_owner(const char *obj_name, Oid role_oid, altered_obj_type obj_type){
+  switch(obj_type){
+    case ALT_FDW:{
+      char* role_name = GetUserNameFromId(role_oid, false);
+      alter_role_super(role_name, true);
 
-  // static allocations to avoid dynamic palloc
-  static const char sql_fdw_template[] =
-    "alter role \"%s\" superuser;\n"
-    "alter foreign data wrapper \"%s\" owner to \"%s\";\n"
-    "alter role \"%s\" nosuperuser;\n";
+      AlterForeignDataWrapperOwner(obj_name, role_oid);
+      CommandCounterIncrement();
 
-  static const char sql_evtrig_template[] =
-    "alter role \"%s\" superuser;\n"
-    "alter event trigger \"%s\" owner to \"%s\";\n"
-    "alter role \"%s\" nosuperuser;\n";
+      alter_role_super(role_name, false);
 
-  static const char sql_sub_template[] =
-    "alter publication \"%s\" owner to \"%s\";\n";
+      break;
+    }
 
-  // max sql length for above templates
-  static const size_t max_sql_len
-        = sizeof (sql_fdw_template) // the fdw template is the largest one
-        + 4 * NAMEDATALEN; // the max size of the 4 identifiers on the fdw template
+    case ALT_PUB:
 
-  char sql[max_sql_len];
+      AlterPublicationOwner(obj_name, role_oid);
+      CommandCounterIncrement();
 
-  switch(obj_type){
-  case ALT_FDW:
-    snprintf(sql,
-             max_sql_len,
-             sql_fdw_template,
-             role_name,
-             obj_name,
-             role_name,
-             role_name);
-    break;
-  case ALT_PUB:
-    snprintf(sql,
-             max_sql_len,
-             sql_sub_template,
-             obj_name,
-             role_name);
-    break;
-  case ALT_EVTRIG:
-    snprintf(sql,
-             max_sql_len,
-             sql_evtrig_template,
-             role_name,
-             obj_name,
-             role_name,
-             role_name);
-    break;
-  }
+      break;
 
-  PushActiveSnapshot(GetTransactionSnapshot());
-  SPI_connect();
+    case ALT_EVTRIG:{
+      char* role_name = GetUserNameFromId(role_oid, false);
 
-  int rc = SPI_execute(sql, false, 0);
-  if (rc != SPI_OK_UTILITY) {
-      elog(ERROR, "SPI_execute failed with error code %d", rc);
-  }
+      alter_role_super(role_name, true);
+
+      AlterEventTriggerOwner(obj_name, role_oid);
+      CommandCounterIncrement();
+
+      alter_role_super(role_name, false);
 
-  SPI_finish();
-  PopActiveSnapshot();
+      break;
+    }
+  }
 }

src/utils.h

@@ -107,7 +107,7 @@ typedef enum {
 
 extern void alter_owner(
     const char *obj_name,
-    const char *role_name,
+    Oid role_oid,
     altered_obj_type obj_type
 );