Re: pg_upgade vs config - Mailing list pgsql-hackers
From | Tom Lane |
---|---|
Subject | Re: pg_upgade vs config |
Date | |
Msg-id | 25402.1475450460@sss.pgh.pa.us Whole thread Raw |
In response to | Re: pg_upgade vs config (Tom Lane <tgl@sss.pgh.pa.us>) |
Responses |
Re: pg_upgade vs config
|
List | pgsql-hackers |
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; }
pgsql-hackers by date: