#2743, avoid re-definition of GUCs during upgrade

git-svn-id: http://svn.osgeo.org/postgis/trunk@14680 b70326c6-7e19-0410-871a-916f4a2858ee
rmunoz · Feb 24, 2016 · 77d3882 · 77d3882
1 parent 25094ae
commit 77d3882
Show file tree

Hide file tree

Showing 4 changed files with 155 additions and 120 deletions.
diff --git a/libpgcommon/lwgeom_pg.c b/libpgcommon/lwgeom_pg.c
@@ -16,8 +16,10 @@
 
 #include <postgres.h>
 #include <fmgr.h>
-#include <executor/spi.h>
 #include <miscadmin.h>
+#include <executor/spi.h>
+#include <utils/guc.h>
+#include <utils/guc_tables.h>
 
 #include "../postgis_config.h"
 #include "liblwgeom.h"
@@ -279,3 +281,81 @@ lwpgerror(const char *fmt, ...)
 
 	va_end(ap);
 }
+
+
+/*
+ * the bare comparison function for GUC names
+ */
+int
+postgis_guc_name_compare(const char *namea, const char *nameb)
+{
+	/*
+	 * The temptation to use strcasecmp() here must be resisted, because the
+	 * array ordering has to remain stable across setlocale() calls. So, build
+	 * our own with a simple ASCII-only downcasing.
+	 */
+	while (*namea && *nameb)
+	{
+		char		cha = *namea++;
+		char		chb = *nameb++;
+
+		if (cha >= 'A' && cha <= 'Z')
+			cha += 'a' - 'A';
+		if (chb >= 'A' && chb <= 'Z')
+			chb += 'a' - 'A';
+		if (cha != chb)
+			return cha - chb;
+	}
+	if (*namea)
+		return 1;				/* a is longer */
+	if (*nameb)
+		return -1;				/* b is longer */
+	return 0;
+}
+
+/*
+ * comparator for qsorting and bsearching guc_variables array
+ */
+int
+postgis_guc_var_compare(const void *a, const void *b)
+{
+	const struct config_generic *confa = *(struct config_generic * const *) a;
+	const struct config_generic *confb = *(struct config_generic * const *) b;
+
+	return postgis_guc_name_compare(confa->name, confb->name);
+}
+
+/* 
+* This is copied from the top half of the find_option function 
+* in postgresql's guc.c. We search the guc_variables for our option.
+* Then we make sure it's not a placeholder. Only then are we sure 
+* we have a potential conflict, of the sort experienced during an 
+* extension upgrade.
+*/
+int
+postgis_guc_find_option(const char *name)
+{
+	const char **key = &name;
+	struct config_generic **res;
+
+	/*
+	 * By equating const char ** with struct config_generic *, we are assuming
+	 * the name field is first in config_generic.
+	 */
+	res = (struct config_generic **) bsearch((void *) &key,
+		 (void *) get_guc_variables(),
+		 GetNumConfigOptions(),
+		 sizeof(struct config_generic *),
+		 postgis_guc_var_compare);
+
+	/* Found nothing? Good */
+	if ( ! res ) return 0;
+
+	/* Hm, you found something, but maybe it's just a placeholder? */
+	/* We'll consider a placehold a "not found" */
+	if ( (*res)->flags & GUC_CUSTOM_PLACEHOLDER )
+		return 0;
+
+	return 1;
+}
+
diff --git a/libpgcommon/lwgeom_pg.h b/libpgcommon/lwgeom_pg.h
@@ -60,6 +60,13 @@ void pg_install_lwgeom_handlers(void);
 
 #endif /* POSTGIS_DEBUG_LEVEL */
 
+/*
+* GUC name search functions stolen from PostgreSQL to
+* support searching for already-defined GUC variables
+*/
+int postgis_guc_name_compare(const char *namea, const char *nameb);
+int postgis_guc_var_compare(const void *a, const void *b);
+int postgis_guc_find_option(const char *name);
 
 /*
  * Standard macro for reporting parser errors to PostgreSQL

diff --git a/postgis/lwgeom_backend_api.c b/postgis/lwgeom_backend_api.c
@@ -113,88 +113,6 @@ static void lwgeom_backend_switch( const char* newvalue, void* extra )
     lwpgerror("Can't find %s geometry backend", newvalue );
 }
 
-
-#include "utils/guc.h"
-#include "utils/guc_tables.h"
-
-
-/*
- * the bare comparison function for GUC names
- */
-static int
-guc_name_compare(const char *namea, const char *nameb)
-{
-	/*
-	 * The temptation to use strcasecmp() here must be resisted, because the
-	 * array ordering has to remain stable across setlocale() calls. So, build
-	 * our own with a simple ASCII-only downcasing.
-	 */
-	while (*namea && *nameb)
-	{
-		char		cha = *namea++;
-		char		chb = *nameb++;
-
-		if (cha >= 'A' && cha <= 'Z')
-			cha += 'a' - 'A';
-		if (chb >= 'A' && chb <= 'Z')
-			chb += 'a' - 'A';
-		if (cha != chb)
-			return cha - chb;
-	}
-	if (*namea)
-		return 1;				/* a is longer */
-	if (*nameb)
-		return -1;				/* b is longer */
-	return 0;
-}
-
-/*
- * comparator for qsorting and bsearching guc_variables array
- */
-static int
-guc_var_compare(const void *a, const void *b)
-{
-	const struct config_generic *confa = *(struct config_generic * const *) a;
-	const struct config_generic *confb = *(struct config_generic * const *) b;
-
-	return guc_name_compare(confa->name, confb->name);
-}
-
-/* 
-* This is copied from the top half of the find_option function 
-* in postgresql's guc.c. We search the guc_variables for our option.
-* Then we make sure it's not a placeholder. Only then are we sure 
-* we have a potential conflict, of the sort experienced during an 
-* extension upgrade.
-*/
-static int
-guc_find_option(const char *name)
-{
-	const char **key = &name;
-	struct config_generic **res;
-
-	/*
-	 * By equating const char ** with struct config_generic *, we are assuming
-	 * the name field is first in config_generic.
-	 */
-	res = (struct config_generic **) bsearch((void *) &key,
-											 (void *) get_guc_variables(),
-											 GetNumConfigOptions(),
-											 sizeof(struct config_generic *),
-											 guc_var_compare);
-
-	/* Found nothing? Good */
-	if ( ! res ) return 0;
-
-	/* Hm, you found something, but maybe it's just a placeholder? */
-	/* We'll consider a placehold a "not found" */
-	if ( (*res)->flags & GUC_CUSTOM_PLACEHOLDER )
-		return 0;
-
-	return 1;
-}
-
-
 void lwgeom_init_backend()
 {
 	/* #2382 Before trying to create a user GUC, make sure */
@@ -210,7 +128,7 @@ void lwgeom_init_backend()
 	/* callback to change which backend is in use by flipping a global variable */
 	/* over. This saves the overhead of looking up the engine every time, at */
 	/* the expense of the extra complexity. */
-	if ( guc_find_option(guc_name) )
+	if ( postgis_guc_find_option(guc_name) )
 	{
 		/* In this narrow case the previously installed GUC is tied to the callback in */
 		/* the previously loaded library. Probably this is happening during an */

diff --git a/raster/rt_pg/rtpostgis.c b/raster/rt_pg/rtpostgis.c
@@ -385,50 +385,80 @@ _PG_init(void) {
 
 	/* Define custom GUC variables. */
 
-	DefineCustomStringVariable(
-		"postgis.gdal_datapath", /* name */
-		"Path to GDAL data files.", /* short_desc */
-		"Physical path to directory containing GDAL data files (sets the GDAL_DATA config option).", /* long_desc */
-		&gdal_datapath, /* valueAddr */
-		NULL, /* bootValue */
-		PGC_SUSET, /* GucContext context */
-		0, /* int flags */
+	if ( postgis_guc_find_option("postgis.gdal_datapath") )
+	{
+		/* In this narrow case the previously installed GUC is tied to the callback in */
+		/* the previously loaded library. Probably this is happening during an */
+		/* upgrade, so the old library is where the callback ties to. */
+		elog(WARNING, "'%s' is already set and cannot be changed until you reconnect", "postgis.gdal_datapath");
+	}
+	else
+	{
+		DefineCustomStringVariable(
+			"postgis.gdal_datapath", /* name */
+			"Path to GDAL data files.", /* short_desc */
+			"Physical path to directory containing GDAL data files (sets the GDAL_DATA config option).", /* long_desc */
+			&gdal_datapath, /* valueAddr */
+			NULL, /* bootValue */
+			PGC_SUSET, /* GucContext context */
+			0, /* int flags */
 #if POSTGIS_PGSQL_VERSION >= 91
-		NULL, /* GucStringCheckHook check_hook */
+			NULL, /* GucStringCheckHook check_hook */
 #endif
-		rtpg_assignHookGDALDataPath, /* GucStringAssignHook assign_hook */
-		NULL  /* GucShowHook show_hook */
-	);
+			rtpg_assignHookGDALDataPath, /* GucStringAssignHook assign_hook */
+			NULL  /* GucShowHook show_hook */
+		);
+	}
 
-	DefineCustomStringVariable(
-		"postgis.gdal_enabled_drivers", /* name */
-		"Enabled GDAL drivers.", /* short_desc */
-		"List of enabled GDAL drivers by short name. To enable/disable all drivers, use 'ENABLE_ALL' or 'DISABLE_ALL' (sets the GDAL_SKIP config option).", /* long_desc */
-		&gdal_enabled_drivers, /* valueAddr */
-		boot_postgis_gdal_enabled_drivers, /* bootValue */
-		PGC_SUSET, /* GucContext context */
-		0, /* int flags */
+	if ( postgis_guc_find_option("postgis.gdal_enabled_drivers") )
+	{
+		/* In this narrow case the previously installed GUC is tied to the callback in */
+		/* the previously loaded library. Probably this is happening during an */
+		/* upgrade, so the old library is where the callback ties to. */
+		elog(WARNING, "'%s' is already set and cannot be changed until you reconnect", "postgis.gdal_enabled_drivers");
+	}
+	else
+	{
+		DefineCustomStringVariable(
+			"postgis.gdal_enabled_drivers", /* name */
+			"Enabled GDAL drivers.", /* short_desc */
+			"List of enabled GDAL drivers by short name. To enable/disable all drivers, use 'ENABLE_ALL' or 'DISABLE_ALL' (sets the GDAL_SKIP config option).", /* long_desc */
+			&gdal_enabled_drivers, /* valueAddr */
+			boot_postgis_gdal_enabled_drivers, /* bootValue */
+			PGC_SUSET, /* GucContext context */
+			0, /* int flags */
 #if POSTGIS_PGSQL_VERSION >= 91
-		NULL, /* GucStringCheckHook check_hook */
+			NULL, /* GucStringCheckHook check_hook */
 #endif
-		rtpg_assignHookGDALEnabledDrivers, /* GucStringAssignHook assign_hook */
-		NULL  /* GucShowHook show_hook */
-	);
+			rtpg_assignHookGDALEnabledDrivers, /* GucStringAssignHook assign_hook */
+			NULL  /* GucShowHook show_hook */
+		);
+	}
 
-	DefineCustomBoolVariable(
-		"postgis.enable_outdb_rasters", /* name */
-		"Enable Out-DB raster bands", /* short_desc */
-		"If true, rasters can access data located outside the database", /* long_desc */
-		&enable_outdb_rasters, /* valueAddr */
-		boot_postgis_enable_outdb_rasters, /* bootValue */
-		PGC_SUSET, /* GucContext context */
-		0, /* int flags */
+	if ( postgis_guc_find_option("postgis.enable_outdb_rasters") )
+	{
+		/* In this narrow case the previously installed GUC is tied to the callback in */
+		/* the previously loaded library. Probably this is happening during an */
+		/* upgrade, so the old library is where the callback ties to. */
+		elog(WARNING, "'%s' is already set and cannot be changed until you reconnect", "postgis.enable_outdb_rasters");
+	}
+	else
+	{
+		DefineCustomBoolVariable(
+			"postgis.enable_outdb_rasters", /* name */
+			"Enable Out-DB raster bands", /* short_desc */
+			"If true, rasters can access data located outside the database", /* long_desc */
+			&enable_outdb_rasters, /* valueAddr */
+			boot_postgis_enable_outdb_rasters, /* bootValue */
+			PGC_SUSET, /* GucContext context */
+			0, /* int flags */
 #if POSTGIS_PGSQL_VERSION >= 91
-		NULL, /* GucStringCheckHook check_hook */
+			NULL, /* GucStringCheckHook check_hook */
 #endif
-		rtpg_assignHookEnableOutDBRasters, /* GucBoolAssignHook assign_hook */
-		NULL  /* GucShowHook show_hook */
-	);
+			rtpg_assignHookEnableOutDBRasters, /* GucBoolAssignHook assign_hook */
+			NULL  /* GucShowHook show_hook */
+		);
+	}
 
 	/* free memory allocations */
 	pfree(boot_postgis_gdal_enabled_drivers);