27.09.2019 15:51, Bruce Momjian wrote:
On Fri, Sep 27, 2019 at 04:22:15PM +0900, Michael Paquier wrote:
On Thu, Sep 26, 2019 at 04:19:38PM -0400, Bruce Momjian wrote:
On Thu, Sep 26, 2019 at 05:16:19PM -0300, Alvaro Herrera wrote:
On 2019-Sep-26, Bruce Momjian wrote:
Well, right now, pg_upgrade --check succeeds, but the upgrade fails.  I
am proposing, at a minimum, that pg_upgrade --check fails in such cases,
Agreed, that should be a minimum fix.
Yes.
Agreed as well here.  At least the latest patch proposed has the merit
to track automatically functions not existing anymore from the
source's version to the target's version, so patching --check offers a
good compromise.  Bruce, are you planning to look more at the patch
posted at [1]?
I did look at it.  It has some TODO items listed in it still, and some
C++ comments, but if everyone likes it I can apply it.

Cool. It seems that everyone agrees on this patch.

I attached the updated version. Now it prints a better error message
and generates an SQL script instead of multiple warnings. The attached test script shows that.

Patches for 10, 11, and 12 slightly differ due to merge conflicts, so I attached multiple versions.

--
Anastasia Lubennikova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

commit dc6d9246c6255bed2bdb2e850a21bb1fc5e0c2fc
Author: Anastasia <a.lubennik...@postgrespro.ru>
Date:   Fri Oct 4 14:39:54 2019 +0300

    pg_upgrade_ACL_check_v2

diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c
index e28b661..bfe9bbb 100644
--- a/src/bin/pg_upgrade/check.c
+++ b/src/bin/pg_upgrade/check.c
@@ -90,6 +90,7 @@ check_and_dump_old_cluster(bool live_check)
 
 	get_loadable_libraries();
 
+	get_catalog_procedures(&old_cluster);
 
 	/*
 	 * Check for various failure cases
@@ -149,6 +150,16 @@ check_new_cluster(void)
 
 	check_loadable_libraries();
 
+	/*
+	 * When restoring non-default ACL from old cluster we may attempt to apply
+	 * GRANT for functions whose signatures have changed significantly.
+	 *
+	 * Compare function lists of old cluster and new cluster to catch
+	 * the incompatibility early and report it to user with a nice error message
+	 */
+	get_catalog_procedures(&new_cluster);
+	check_catalog_procedures(&old_cluster, &new_cluster);
+
 	if (user_opts.transfer_mode == TRANSFER_MODE_LINK)
 		check_hard_link();
 
diff --git a/src/bin/pg_upgrade/function.c b/src/bin/pg_upgrade/function.c
index 063a94f..0fce929 100644
--- a/src/bin/pg_upgrade/function.c
+++ b/src/bin/pg_upgrade/function.c
@@ -13,6 +13,7 @@
 
 #include "access/transam.h"
 #include "catalog/pg_language.h"
+#include "fe_utils/string_utils.h"
 
 
 /*
@@ -275,3 +276,204 @@ check_loadable_libraries(void)
 	else
 		check_ok();
 }
+
+/*
+ * qsort comparator for procedure signatures
+ */
+static int
+proc_compare_sig(const void *p1, const void *p2)
+{
+	ProcInfo *proc1 = (ProcInfo *) p1;
+	ProcInfo *proc2 = (ProcInfo *) p2;
+
+	return strcmp(proc1->procsig, proc2->procsig);
+}
+
+/*
+ * Fetch the signatures and ACL of cluster's system procedures.
+ */
+void
+get_catalog_procedures(ClusterInfo *cluster)
+{
+	int			dbnum;
+
+	/*
+	 * Fetch all procedure signatures and ACL.
+	 * Each procedure may have different ACL in different database.
+	 */
+	for (dbnum = 0; dbnum < cluster->dbarr.ndbs; dbnum++)
+	{
+		DbInfo	   *dbinfo = &cluster->dbarr.dbs[dbnum];
+		PGconn	   *conn = connectToServer(cluster, dbinfo->db_name);
+		PGresult   *res;
+		int			num_procs;
+		int			rowno;
+
+		/*
+		 * Fetch procedure signatures and ACL of functions that have
+		 * some non default ACL.
+		 */
+		if (cluster->major_version >= 110000)
+		{
+			res = executeQueryOrDie(conn,
+						"select proname::text || '('"
+						" || pg_get_function_arguments(pg_proc.oid)::text"
+						" || ')' as funsig,"
+						" array (SELECT unnest(pg_proc.proacl)"
+						" EXCEPT SELECT unnest(pg_init_privs.initprivs))"
+						" from pg_proc join pg_init_privs"
+						" on pg_proc.oid = pg_init_privs.objoid"
+						" where prokind='f' and proacl != initprivs;");
+		}
+		else
+		{
+			res = executeQueryOrDie(conn,
+					"select proname::text || '('"
+					" || pg_get_function_arguments(pg_proc.oid)::text"
+					" || ')' as funsig,"
+					" array (SELECT unnest(pg_proc.proacl)"
+					" EXCEPT SELECT unnest(pg_init_privs.initprivs))"
+					" from pg_proc join pg_init_privs"
+					" on pg_proc.oid = pg_init_privs.objoid"
+					" where proisagg = false and proacl != initprivs;");
+		}
+
+		num_procs = PQntuples(res);
+		dbinfo->proc_arr.nprocs = num_procs;
+		dbinfo->proc_arr.procs = (ProcInfo *) pg_malloc(sizeof(ProcInfo) * num_procs);
+
+		for (rowno = 0; rowno < num_procs; rowno++)
+		{
+			ProcInfo    *curr = &dbinfo->proc_arr.procs[rowno];
+			char	   *procsig = PQgetvalue(res, rowno, 0);
+			char	   *procacl = PQgetvalue(res, rowno, 1);
+
+			curr->procsig = pg_strdup(procsig);
+			curr->procacl = pg_strdup(procacl);
+		}
+
+		qsort((void *) dbinfo->proc_arr.procs, dbinfo->proc_arr.nprocs,
+			  sizeof(ProcInfo), proc_compare_sig);
+
+		PQclear(res);
+		PQfinish(conn);
+	}
+}
+
+/* helper for check_catalog_procedures() */
+static void
+log_incompatible_procedure(FILE **script, char *output_path,
+						   DbInfo *olddbinfo, bool *new_db,
+						   ProcInfo *curr)
+{
+	char	  **aclitems;
+	int			naclitems;
+	int			i;
+
+	if (*script == NULL && (*script = fopen_priv(output_path, "w")) == NULL)
+		pg_fatal("could not open file \"%s\": %s\n", output_path, strerror(errno));
+
+	if (*new_db)
+	{
+		fprintf(*script, _("\\c %s \n"), olddbinfo->db_name);
+		*new_db = false;
+	}
+
+	if (!parsePGArray(curr->procacl, &aclitems, &naclitems))
+	{
+		if (aclitems)
+			free(aclitems);
+		pg_fatal("check_catalog_procedures failed \n");
+	}
+
+	/* generate REVOKE command for each ACL on this function */
+	for (i = 0; i < naclitems; i++)
+	{
+		char *pos = strchr(aclitems[i], '=');
+		*pos = '\0';
+
+		if (pos != aclitems[i])
+			fprintf(*script, _("REVOKE ALL ON FUNCTION %s FROM %s;\n"),
+					curr->procsig, aclitems[i]);
+		else
+			fprintf(*script, _("REVOKE ALL ON FUNCTION %s FROM PUBLIC;\n"),
+					curr->procsig);
+	}
+}
+
+/*
+ * Check API changes in pg_proc between old_cluster and new_cluster.
+ * Report functions that only exist in old_cluster.
+ *
+ * NOTE It's vital to call it after check_databases_are_compatible(),
+ * because we rely on correct database order.
+ */
+void
+check_catalog_procedures(ClusterInfo *old_cluster, ClusterInfo *new_cluster)
+{
+	int			dbnum;
+	FILE	   *script = NULL;
+	bool		found = false;
+	char		output_path[MAXPGPATH];
+	snprintf(output_path, sizeof(output_path), "catalog_procedures.sql");
+
+	prep_status("Checking for system functions API compatibility\n");
+
+	for (dbnum = 0; dbnum < old_cluster->dbarr.ndbs; dbnum++)
+	{
+		DbInfo	   *olddbinfo = &old_cluster->dbarr.dbs[dbnum];
+		DbInfo	   *newdbinfo = &new_cluster->dbarr.dbs[dbnum];
+		int i, j;
+		bool new_db = true;
+
+		if (strcmp(olddbinfo->db_name, newdbinfo->db_name) != 0)
+			pg_log(PG_FATAL, "check_catalog_procedures failed \n");
+
+		i = j = 0;
+		while (i < olddbinfo->proc_arr.nprocs && j < newdbinfo->proc_arr.nprocs)
+		{
+			ProcInfo    *oldcurr = &olddbinfo->proc_arr.procs[i];
+			ProcInfo    *newcurr = &newdbinfo->proc_arr.procs[j];
+			int			result = strcmp(oldcurr->procsig, newcurr->procsig);
+
+			if (result == 0) /* The function exists in both clusters */
+			{
+				i++;
+				j++;
+			}
+			else if (result < 0) /* The function exists only in the old cluster */
+			{
+				log_incompatible_procedure(&script, output_path,
+									olddbinfo, &new_db, oldcurr);
+				found = true;
+				i++;
+			}
+			else if (result > 0)  /* The function exists only in the new cluster */
+				j++;
+		}
+
+		/* handle tail of old_cluster proc list */
+		while (i < olddbinfo->proc_arr.nprocs)
+		{
+			ProcInfo    *oldcurr = &olddbinfo->proc_arr.procs[i];
+			/* The function exists only in the old cluster */
+			log_incompatible_procedure(&script, output_path,
+									olddbinfo, &new_db, oldcurr);
+			found = true;
+
+			i++;
+		}
+	}
+
+	if (found)
+	{
+		fclose(script);
+		pg_log(PG_REPORT, "fatal\n");
+		pg_fatal("Your installation contains non-default ACL on system procedures,\n"
+				 "which API have changed between versions. To perform upgrade, reset ACL to default.\n"
+				 "Script to REVOKE ALL on problem fuctions is in the file:\n"
+				 "    %s\n\n", output_path);
+	}
+	else
+		check_ok();
+}
diff --git a/src/bin/pg_upgrade/pg_upgrade.h b/src/bin/pg_upgrade/pg_upgrade.h
index dccc3e3..b33862c 100644
--- a/src/bin/pg_upgrade/pg_upgrade.h
+++ b/src/bin/pg_upgrade/pg_upgrade.h
@@ -155,6 +155,19 @@ typedef struct
 	int			nrels;
 } RelInfoArr;
 
+typedef struct
+{
+	char	   *procsig;		/* procedure signature */
+	char	   *procacl;		/* ACL */
+} ProcInfo;
+
+typedef struct
+{
+	ProcInfo    *procs;
+	int			nprocs;
+} ProcInfoArr;
+
+
 /*
  * The following structure represents a relation mapping.
  */
@@ -191,6 +204,7 @@ typedef struct
 	char	   *db_ctype;
 	int			db_encoding;
 	RelInfoArr	rel_arr;		/* array of all user relinfos */
+	ProcInfoArr proc_arr;	/* array of system procedures */
 } DbInfo;
 
 typedef struct
@@ -382,6 +396,9 @@ void		check_hard_link(void);
 
 void		get_loadable_libraries(void);
 void		check_loadable_libraries(void);
+void		get_catalog_procedures(ClusterInfo *cluster);
+void		check_catalog_procedures(ClusterInfo *old_cluster,
+									 ClusterInfo *new_cluster);
 
 /* info.c */
 
commit 515d895c599cb159b1f06d23b2027f72be6c5320
Author: Anastasia <a.lubennik...@postgrespro.ru>
Date:   Thu Oct 3 13:50:54 2019 +0300

    pg_upgrade_ACL_check_v2

diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c
index b4cf6da..1ac3436 100644
--- a/src/bin/pg_upgrade/check.c
+++ b/src/bin/pg_upgrade/check.c
@@ -90,6 +90,7 @@ check_and_dump_old_cluster(bool live_check)
 
 	get_loadable_libraries();
 
+	get_catalog_procedures(&old_cluster);
 
 	/*
 	 * Check for various failure cases
@@ -149,6 +150,16 @@ check_new_cluster(void)
 
 	check_loadable_libraries();
 
+	/*
+	 * When restoring non-default ACL from old cluster we may attempt to apply
+	 * GRANT for functions whose signatures have changed significantly.
+	 *
+	 * Compare function lists of old cluster and new cluster to catch
+	 * the incompatibility early and report it to user with a nice error message
+	 */
+	get_catalog_procedures(&new_cluster);
+	check_catalog_procedures(&old_cluster, &new_cluster);
+
 	if (user_opts.transfer_mode == TRANSFER_MODE_LINK)
 		check_hard_link();
 
diff --git a/src/bin/pg_upgrade/function.c b/src/bin/pg_upgrade/function.c
index 03fd155..8a43bf6 100644
--- a/src/bin/pg_upgrade/function.c
+++ b/src/bin/pg_upgrade/function.c
@@ -13,6 +13,7 @@
 
 #include "access/transam.h"
 #include "catalog/pg_language_d.h"
+#include "fe_utils/string_utils.h"
 
 
 /*
@@ -275,3 +276,204 @@ check_loadable_libraries(void)
 	else
 		check_ok();
 }
+
+/*
+ * qsort comparator for procedure signatures
+ */
+static int
+proc_compare_sig(const void *p1, const void *p2)
+{
+	ProcInfo *proc1 = (ProcInfo *) p1;
+	ProcInfo *proc2 = (ProcInfo *) p2;
+
+	return strcmp(proc1->procsig, proc2->procsig);
+}
+
+/*
+ * Fetch the signatures and ACL of cluster's system procedures.
+ */
+void
+get_catalog_procedures(ClusterInfo *cluster)
+{
+	int			dbnum;
+
+	/*
+	 * Fetch all procedure signatures and ACL.
+	 * Each procedure may have different ACL in different database.
+	 */
+	for (dbnum = 0; dbnum < cluster->dbarr.ndbs; dbnum++)
+	{
+		DbInfo	   *dbinfo = &cluster->dbarr.dbs[dbnum];
+		PGconn	   *conn = connectToServer(cluster, dbinfo->db_name);
+		PGresult   *res;
+		int			num_procs;
+		int			rowno;
+
+		/*
+		 * Fetch procedure signatures and ACL of functions that have
+		 * some non default ACL.
+		 */
+		if (cluster->major_version >= 110000)
+		{
+			res = executeQueryOrDie(conn,
+						"select proname::text || '('"
+						" || pg_get_function_arguments(pg_proc.oid)::text"
+						" || ')' as funsig,"
+						" array (SELECT unnest(pg_proc.proacl)"
+						" EXCEPT SELECT unnest(pg_init_privs.initprivs))"
+						" from pg_proc join pg_init_privs"
+						" on pg_proc.oid = pg_init_privs.objoid"
+						" where prokind='f' and proacl != initprivs;");
+		}
+		else
+		{
+			res = executeQueryOrDie(conn,
+					"select proname::text || '('"
+					" || pg_get_function_arguments(pg_proc.oid)::text"
+					" || ')' as funsig,"
+					" array (SELECT unnest(pg_proc.proacl)"
+					" EXCEPT SELECT unnest(pg_init_privs.initprivs))"
+					" from pg_proc join pg_init_privs"
+					" on pg_proc.oid = pg_init_privs.objoid"
+					" where proisagg = false and proacl != initprivs;");
+		}
+
+		num_procs = PQntuples(res);
+		dbinfo->proc_arr.nprocs = num_procs;
+		dbinfo->proc_arr.procs = (ProcInfo *) pg_malloc(sizeof(ProcInfo) * num_procs);
+
+		for (rowno = 0; rowno < num_procs; rowno++)
+		{
+			ProcInfo    *curr = &dbinfo->proc_arr.procs[rowno];
+			char	   *procsig = PQgetvalue(res, rowno, 0);
+			char	   *procacl = PQgetvalue(res, rowno, 1);
+
+			curr->procsig = pg_strdup(procsig);
+			curr->procacl = pg_strdup(procacl);
+		}
+
+		qsort((void *) dbinfo->proc_arr.procs, dbinfo->proc_arr.nprocs,
+			  sizeof(ProcInfo), proc_compare_sig);
+
+		PQclear(res);
+		PQfinish(conn);
+	}
+}
+
+/* helper for check_catalog_procedures() */
+static void
+log_incompatible_procedure(FILE **script, char *output_path,
+						   DbInfo *olddbinfo, bool *new_db,
+						   ProcInfo *curr)
+{
+	char	  **aclitems;
+	int			naclitems;
+	int			i;
+
+	if (*script == NULL && (*script = fopen_priv(output_path, "w")) == NULL)
+		pg_fatal("could not open file \"%s\": %s\n", output_path, strerror(errno));
+
+	if (*new_db)
+	{
+		fprintf(*script, _("\\c %s \n"), olddbinfo->db_name);
+		*new_db = false;
+	}
+
+	if (!parsePGArray(curr->procacl, &aclitems, &naclitems))
+	{
+		if (aclitems)
+			free(aclitems);
+		pg_fatal("check_catalog_procedures failed \n");
+	}
+
+	/* generate REVOKE command for each ACL on this function */
+	for (i = 0; i < naclitems; i++)
+	{
+		char *pos = strchr(aclitems[i], '=');
+		*pos = '\0';
+
+		if (pos != aclitems[i])
+			fprintf(*script, _("REVOKE ALL ON FUNCTION %s FROM %s;\n"),
+					curr->procsig, aclitems[i]);
+		else
+			fprintf(*script, _("REVOKE ALL ON FUNCTION %s FROM PUBLIC;\n"),
+					curr->procsig);
+	}
+}
+
+/*
+ * Check API changes in pg_proc between old_cluster and new_cluster.
+ * Report functions that only exist in old_cluster.
+ *
+ * NOTE It's vital to call it after check_databases_are_compatible(),
+ * because we rely on correct database order.
+ */
+void
+check_catalog_procedures(ClusterInfo *old_cluster, ClusterInfo *new_cluster)
+{
+	int			dbnum;
+	FILE	   *script = NULL;
+	bool		found = false;
+	char		output_path[MAXPGPATH];
+	snprintf(output_path, sizeof(output_path), "catalog_procedures.sql");
+
+	prep_status("Checking for system functions API compatibility\n");
+
+	for (dbnum = 0; dbnum < old_cluster->dbarr.ndbs; dbnum++)
+	{
+		DbInfo	   *olddbinfo = &old_cluster->dbarr.dbs[dbnum];
+		DbInfo	   *newdbinfo = &new_cluster->dbarr.dbs[dbnum];
+		int i, j;
+		bool new_db = true;
+
+		if (strcmp(olddbinfo->db_name, newdbinfo->db_name) != 0)
+			pg_log(PG_FATAL, "check_catalog_procedures failed \n");
+
+		i = j = 0;
+		while (i < olddbinfo->proc_arr.nprocs && j < newdbinfo->proc_arr.nprocs)
+		{
+			ProcInfo    *oldcurr = &olddbinfo->proc_arr.procs[i];
+			ProcInfo    *newcurr = &newdbinfo->proc_arr.procs[j];
+			int			result = strcmp(oldcurr->procsig, newcurr->procsig);
+
+			if (result == 0) /* The function exists in both clusters */
+			{
+				i++;
+				j++;
+			}
+			else if (result < 0) /* The function exists only in the old cluster */
+			{
+				log_incompatible_procedure(&script, output_path,
+									olddbinfo, &new_db, oldcurr);
+				found = true;
+				i++;
+			}
+			else if (result > 0)  /* The function exists only in the new cluster */
+				j++;
+		}
+
+		/* handle tail of old_cluster proc list */
+		while (i < olddbinfo->proc_arr.nprocs)
+		{
+			ProcInfo    *oldcurr = &olddbinfo->proc_arr.procs[i];
+			/* The function exists only in the old cluster */
+			log_incompatible_procedure(&script, output_path,
+									olddbinfo, &new_db, oldcurr);
+			found = true;
+
+			i++;
+		}
+	}
+
+	if (found)
+	{
+		fclose(script);
+		pg_log(PG_REPORT, "fatal\n");
+		pg_fatal("Your installation contains non-default ACL on system procedures,\n"
+				 "which API have changed between versions. To perform upgrade, reset ACL to default.\n"
+				 "Script to REVOKE ALL on problem fuctions is in the file:\n"
+				 "    %s\n\n", output_path);
+	}
+	else
+		check_ok();
+}
diff --git a/src/bin/pg_upgrade/pg_upgrade.h b/src/bin/pg_upgrade/pg_upgrade.h
index 7e5e971..5ed4f8e 100644
--- a/src/bin/pg_upgrade/pg_upgrade.h
+++ b/src/bin/pg_upgrade/pg_upgrade.h
@@ -155,6 +155,19 @@ typedef struct
 	int			nrels;
 } RelInfoArr;
 
+typedef struct
+{
+	char	   *procsig;		/* procedure signature */
+	char	   *procacl;		/* ACL */
+} ProcInfo;
+
+typedef struct
+{
+	ProcInfo    *procs;
+	int			nprocs;
+} ProcInfoArr;
+
+
 /*
  * The following structure represents a relation mapping.
  */
@@ -191,6 +204,7 @@ typedef struct
 	char	   *db_ctype;
 	int			db_encoding;
 	RelInfoArr	rel_arr;		/* array of all user relinfos */
+	ProcInfoArr proc_arr;	/* array of system procedures */
 } DbInfo;
 
 typedef struct
@@ -382,6 +396,9 @@ void		check_hard_link(void);
 
 void		get_loadable_libraries(void);
 void		check_loadable_libraries(void);
+void		get_catalog_procedures(ClusterInfo *cluster);
+void		check_catalog_procedures(ClusterInfo *old_cluster,
+									 ClusterInfo *new_cluster);
 
 /* info.c */
 
commit 2a13cd12afd28368a26898dd1e19f17b08bb1c45
Author: Anastasia <a.lubennik...@postgrespro.ru>
Date:   Fri Oct 4 14:43:45 2019 +0300

    pg_upgrade_ACL_check_v2

diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c
index 617270f..1c7a34a 100644
--- a/src/bin/pg_upgrade/check.c
+++ b/src/bin/pg_upgrade/check.c
@@ -91,6 +91,7 @@ check_and_dump_old_cluster(bool live_check)
 
 	get_loadable_libraries();
 
+	get_catalog_procedures(&old_cluster);
 
 	/*
 	 * Check for various failure cases
@@ -157,6 +158,16 @@ check_new_cluster(void)
 
 	check_loadable_libraries();
 
+	/*
+	 * When restoring non-default ACL from old cluster we may attempt to apply
+	 * GRANT for functions whose signatures have changed significantly.
+	 *
+	 * Compare function lists of old cluster and new cluster to catch
+	 * the incompatibility early and report it to user with a nice error message
+	 */
+	get_catalog_procedures(&new_cluster);
+	check_catalog_procedures(&old_cluster, &new_cluster);
+
 	switch (user_opts.transfer_mode)
 	{
 		case TRANSFER_MODE_CLONE:
diff --git a/src/bin/pg_upgrade/function.c b/src/bin/pg_upgrade/function.c
index 0c66d1c..a5f8232 100644
--- a/src/bin/pg_upgrade/function.c
+++ b/src/bin/pg_upgrade/function.c
@@ -13,6 +13,7 @@
 
 #include "access/transam.h"
 #include "catalog/pg_language_d.h"
+#include "fe_utils/string_utils.h"
 
 
 /*
@@ -275,3 +276,204 @@ check_loadable_libraries(void)
 	else
 		check_ok();
 }
+
+/*
+ * qsort comparator for procedure signatures
+ */
+static int
+proc_compare_sig(const void *p1, const void *p2)
+{
+	ProcInfo *proc1 = (ProcInfo *) p1;
+	ProcInfo *proc2 = (ProcInfo *) p2;
+
+	return strcmp(proc1->procsig, proc2->procsig);
+}
+
+/*
+ * Fetch the signatures and ACL of cluster's system procedures.
+ */
+void
+get_catalog_procedures(ClusterInfo *cluster)
+{
+	int			dbnum;
+
+	/*
+	 * Fetch all procedure signatures and ACL.
+	 * Each procedure may have different ACL in different database.
+	 */
+	for (dbnum = 0; dbnum < cluster->dbarr.ndbs; dbnum++)
+	{
+		DbInfo	   *dbinfo = &cluster->dbarr.dbs[dbnum];
+		PGconn	   *conn = connectToServer(cluster, dbinfo->db_name);
+		PGresult   *res;
+		int			num_procs;
+		int			rowno;
+
+		/*
+		 * Fetch procedure signatures and ACL of functions that have
+		 * some non default ACL.
+		 */
+		if (cluster->major_version >= 110000)
+		{
+			res = executeQueryOrDie(conn,
+						"select proname::text || '('"
+						" || pg_get_function_arguments(pg_proc.oid)::text"
+						" || ')' as funsig,"
+						" array (SELECT unnest(pg_proc.proacl)"
+						" EXCEPT SELECT unnest(pg_init_privs.initprivs))"
+						" from pg_proc join pg_init_privs"
+						" on pg_proc.oid = pg_init_privs.objoid"
+						" where prokind='f' and proacl != initprivs;");
+		}
+		else
+		{
+			res = executeQueryOrDie(conn,
+					"select proname::text || '('"
+					" || pg_get_function_arguments(pg_proc.oid)::text"
+					" || ')' as funsig,"
+					" array (SELECT unnest(pg_proc.proacl)"
+					" EXCEPT SELECT unnest(pg_init_privs.initprivs))"
+					" from pg_proc join pg_init_privs"
+					" on pg_proc.oid = pg_init_privs.objoid"
+					" where proisagg = false and proacl != initprivs;");
+		}
+
+		num_procs = PQntuples(res);
+		dbinfo->proc_arr.nprocs = num_procs;
+		dbinfo->proc_arr.procs = (ProcInfo *) pg_malloc(sizeof(ProcInfo) * num_procs);
+
+		for (rowno = 0; rowno < num_procs; rowno++)
+		{
+			ProcInfo    *curr = &dbinfo->proc_arr.procs[rowno];
+			char	   *procsig = PQgetvalue(res, rowno, 0);
+			char	   *procacl = PQgetvalue(res, rowno, 1);
+
+			curr->procsig = pg_strdup(procsig);
+			curr->procacl = pg_strdup(procacl);
+		}
+
+		qsort((void *) dbinfo->proc_arr.procs, dbinfo->proc_arr.nprocs,
+			  sizeof(ProcInfo), proc_compare_sig);
+
+		PQclear(res);
+		PQfinish(conn);
+	}
+}
+
+/* helper for check_catalog_procedures() */
+static void
+log_incompatible_procedure(FILE **script, char *output_path,
+						   DbInfo *olddbinfo, bool *new_db,
+						   ProcInfo *curr)
+{
+	char	  **aclitems;
+	int			naclitems;
+	int			i;
+
+	if (*script == NULL && (*script = fopen_priv(output_path, "w")) == NULL)
+		pg_fatal("could not open file \"%s\": %s\n", output_path, strerror(errno));
+
+	if (*new_db)
+	{
+		fprintf(*script, _("\\c %s \n"), olddbinfo->db_name);
+		*new_db = false;
+	}
+
+	if (!parsePGArray(curr->procacl, &aclitems, &naclitems))
+	{
+		if (aclitems)
+			free(aclitems);
+		pg_fatal("check_catalog_procedures failed \n");
+	}
+
+	/* generate REVOKE command for each ACL on this function */
+	for (i = 0; i < naclitems; i++)
+	{
+		char *pos = strchr(aclitems[i], '=');
+		*pos = '\0';
+
+		if (pos != aclitems[i])
+			fprintf(*script, _("REVOKE ALL ON FUNCTION %s FROM %s;\n"),
+					curr->procsig, aclitems[i]);
+		else
+			fprintf(*script, _("REVOKE ALL ON FUNCTION %s FROM PUBLIC;\n"),
+					curr->procsig);
+	}
+}
+
+/*
+ * Check API changes in pg_proc between old_cluster and new_cluster.
+ * Report functions that only exist in old_cluster.
+ *
+ * NOTE It's vital to call it after check_databases_are_compatible(),
+ * because we rely on correct database order.
+ */
+void
+check_catalog_procedures(ClusterInfo *old_cluster, ClusterInfo *new_cluster)
+{
+	int			dbnum;
+	FILE	   *script = NULL;
+	bool		found = false;
+	char		output_path[MAXPGPATH];
+	snprintf(output_path, sizeof(output_path), "catalog_procedures.sql");
+
+	prep_status("Checking for system functions API compatibility\n");
+
+	for (dbnum = 0; dbnum < old_cluster->dbarr.ndbs; dbnum++)
+	{
+		DbInfo	   *olddbinfo = &old_cluster->dbarr.dbs[dbnum];
+		DbInfo	   *newdbinfo = &new_cluster->dbarr.dbs[dbnum];
+		int i, j;
+		bool new_db = true;
+
+		if (strcmp(olddbinfo->db_name, newdbinfo->db_name) != 0)
+			pg_log(PG_FATAL, "check_catalog_procedures failed \n");
+
+		i = j = 0;
+		while (i < olddbinfo->proc_arr.nprocs && j < newdbinfo->proc_arr.nprocs)
+		{
+			ProcInfo    *oldcurr = &olddbinfo->proc_arr.procs[i];
+			ProcInfo    *newcurr = &newdbinfo->proc_arr.procs[j];
+			int			result = strcmp(oldcurr->procsig, newcurr->procsig);
+
+			if (result == 0) /* The function exists in both clusters */
+			{
+				i++;
+				j++;
+			}
+			else if (result < 0) /* The function exists only in the old cluster */
+			{
+				log_incompatible_procedure(&script, output_path,
+									olddbinfo, &new_db, oldcurr);
+				found = true;
+				i++;
+			}
+			else if (result > 0)  /* The function exists only in the new cluster */
+				j++;
+		}
+
+		/* handle tail of old_cluster proc list */
+		while (i < olddbinfo->proc_arr.nprocs)
+		{
+			ProcInfo    *oldcurr = &olddbinfo->proc_arr.procs[i];
+			/* The function exists only in the old cluster */
+			log_incompatible_procedure(&script, output_path,
+									olddbinfo, &new_db, oldcurr);
+			found = true;
+
+			i++;
+		}
+	}
+
+	if (found)
+	{
+		fclose(script);
+		pg_log(PG_REPORT, "fatal\n");
+		pg_fatal("Your installation contains non-default ACL on system procedures,\n"
+				 "which API have changed between versions. To perform upgrade, reset ACL to default.\n"
+				 "Script to REVOKE ALL on problem fuctions is in the file:\n"
+				 "    %s\n\n", output_path);
+	}
+	else
+		check_ok();
+}
diff --git a/src/bin/pg_upgrade/pg_upgrade.h b/src/bin/pg_upgrade/pg_upgrade.h
index 5d31750..97db9e5 100644
--- a/src/bin/pg_upgrade/pg_upgrade.h
+++ b/src/bin/pg_upgrade/pg_upgrade.h
@@ -155,6 +155,19 @@ typedef struct
 	int			nrels;
 } RelInfoArr;
 
+typedef struct
+{
+	char	   *procsig;		/* procedure signature */
+	char	   *procacl;		/* ACL */
+} ProcInfo;
+
+typedef struct
+{
+	ProcInfo    *procs;
+	int			nprocs;
+} ProcInfoArr;
+
+
 /*
  * The following structure represents a relation mapping.
  */
@@ -191,6 +204,7 @@ typedef struct
 	char	   *db_ctype;
 	int			db_encoding;
 	RelInfoArr	rel_arr;		/* array of all user relinfos */
+	ProcInfoArr proc_arr;	/* array of system procedures */
 } DbInfo;
 
 typedef struct
@@ -392,6 +406,9 @@ void		check_hard_link(void);
 
 void		get_loadable_libraries(void);
 void		check_loadable_libraries(void);
+void		get_catalog_procedures(ClusterInfo *cluster);
+void		check_catalog_procedures(ClusterInfo *old_cluster,
+									 ClusterInfo *new_cluster);
 
 /* info.c */
 

Attachment: pg_upgrade_ACL_test.sh
Description: application/shellscript

Reply via email to