I wrote:
> It occurs to me that a back-patchable workaround for this would be to
> make get_loadable_libraries sort the library names in order by length
> (and I guess we might as well sort same-length names alphabetically).
> This would for example guarantee that hstore_plpython is probed after
> both hstore and plpython. Admittedly, this is a kluge of the first
> water. But I see no prospect of back-patching any real fix, and it
> would definitely be better if pg_upgrade didn't fail on these modules.
I've tested the attached and verified that it allows pg_upgrade'ing
of the hstore_plpython regression DB --- or, if I reverse the sort
order, that it reproducibly fails. I propose back-patching this
at least as far as 9.5, where the transform modules came in. It might
be a good idea to go all the way back, just so that the behavior is
predictable.
regards, tom lane
diff --git a/src/bin/pg_upgrade/function.c b/src/bin/pg_upgrade/function.c
index b432b54..5435bff 100644
*** a/src/bin/pg_upgrade/function.c
--- b/src/bin/pg_upgrade/function.c
***************
*** 15,20 ****
--- 15,44 ----
/*
+ * qsort comparator for pointers to library names
+ *
+ * We sort first by name length, then alphabetically for names of the same
+ * length. This is to ensure that, eg, "hstore_plpython" sorts after both
+ * "hstore" and "plpython"; otherwise transform modules will probably fail
+ * their LOAD tests. (The backend ought to cope with that consideration,
+ * but it doesn't yet, and even when it does it'd be a good idea to have
+ * a predictable order of probing here.)
+ */
+ static int
+ library_name_compare(const void *p1, const void *p2)
+ {
+ const char *str1 = *(const char *const *) p1;
+ const char *str2 = *(const char *const *) p2;
+ int slen1 = strlen(str1);
+ int slen2 = strlen(str2);
+
+ if (slen1 != slen2)
+ return slen1 - slen2;
+ return strcmp(str1, str2);
+ }
+
+
+ /*
* get_loadable_libraries()
*
* Fetch the names of all old libraries containing C-language functions.
*************** get_loadable_libraries(void)
*** 38,47 ****
PGconn *conn = connectToServer(&old_cluster, active_db->db_name);
/*
! * Fetch all libraries referenced in this DB. We can't exclude the
! * "pg_catalog" schema because, while such functions are not
! * explicitly dumped by pg_dump, they do reference implicit objects
! * that pg_dump does dump, e.g. CREATE LANGUAGE plperl.
*/
ress[dbnum] = executeQueryOrDie(conn,
"SELECT DISTINCT probin "
--- 62,68 ----
PGconn *conn = connectToServer(&old_cluster, active_db->db_name);
/*
! * Fetch all libraries referenced in this DB.
*/
ress[dbnum] = executeQueryOrDie(conn,
"SELECT DISTINCT probin "
*************** get_loadable_libraries(void)
*** 69,76 ****
res = executeQueryOrDie(conn,
"SELECT 1 "
! "FROM pg_catalog.pg_proc JOIN pg_namespace "
! " ON pronamespace = pg_namespace.oid "
"WHERE proname = 'plpython_call_handler' AND "
"nspname = 'public' AND "
"prolang = 13 /* C */ AND "
--- 90,98 ----
res = executeQueryOrDie(conn,
"SELECT 1 "
! "FROM pg_catalog.pg_proc p "
! " JOIN pg_catalog.pg_namespace n "
! " ON pronamespace = n.oid "
"WHERE proname = 'plpython_call_handler' AND "
"nspname = 'public' AND "
"prolang = 13 /* C */ AND "
*************** get_loadable_libraries(void)
*** 112,124 ****
if (found_public_plpython_handler)
pg_fatal("Remove the problem functions from the old cluster to continue.\n");
- /* Allocate what's certainly enough space */
- os_info.libraries = (char **) pg_malloc(totaltups * sizeof(char *));
-
/*
! * Now remove duplicates across DBs. This is pretty inefficient code, but
! * there probably aren't enough entries to matter.
*/
totaltups = 0;
for (dbnum = 0; dbnum < old_cluster.dbarr.ndbs; dbnum++)
--- 134,151 ----
if (found_public_plpython_handler)
pg_fatal("Remove the problem functions from the old cluster to continue.\n");
/*
! * Now we want to remove duplicates across DBs and sort the library names
! * into order. This avoids multiple probes of the same library, and
! * ensures that libraries are probed in a consistent order, which is
! * important for reproducible behavior if one library depends on another.
! *
! * First transfer all the names into one array, then sort, then remove
! * duplicates. Note: we strdup each name in the first loop so that we can
! * safely clear the PGresults in the same loop. This is a bit wasteful
! * but it's unlikely there are enough names to matter.
*/
+ os_info.libraries = (char **) pg_malloc(totaltups * sizeof(char *));
totaltups = 0;
for (dbnum = 0; dbnum < old_cluster.dbarr.ndbs; dbnum++)
*************** get_loadable_libraries(void)
*** 131,157 ****
for (rowno = 0; rowno < ntups; rowno++)
{
char *lib = PQgetvalue(res, rowno, 0);
- bool dup = false;
- int n;
! for (n = 0; n < totaltups; n++)
! {
! if (strcmp(lib, os_info.libraries[n]) == 0)
! {
! dup = true;
! break;
! }
! }
! if (!dup)
! os_info.libraries[totaltups++] = pg_strdup(lib);
}
-
PQclear(res);
}
- os_info.num_libraries = totaltups;
-
pg_free(ress);
}
--- 158,191 ----
for (rowno = 0; rowno < ntups; rowno++)
{
char *lib = PQgetvalue(res, rowno, 0);
! os_info.libraries[totaltups++] = pg_strdup(lib);
}
PQclear(res);
}
pg_free(ress);
+
+ if (totaltups > 1)
+ {
+ int i,
+ lastnondup;
+
+ qsort((void *) os_info.libraries, totaltups, sizeof(char *),
+ library_name_compare);
+
+ for (i = 1, lastnondup = 0; i < totaltups; i++)
+ {
+ if (strcmp(os_info.libraries[i],
+ os_info.libraries[lastnondup]) != 0)
+ os_info.libraries[++lastnondup] = os_info.libraries[i];
+ else
+ pg_free(os_info.libraries[i]);
+ }
+ totaltups = lastnondup + 1;
+ }
+
+ os_info.num_libraries = totaltups;
}
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
