From 77d3882cc86f5080e9fe2a3382e8e2f5455ae51b Mon Sep 17 00:00:00 2001 From: Paul Ramsey Date: Wed, 24 Feb 2016 15:19:04 +0000 Subject: [PATCH] #2743, avoid re-definition of GUCs during upgrade git-svn-id: http://svn.osgeo.org/postgis/trunk@14680 b70326c6-7e19-0410-871a-916f4a2858ee --- libpgcommon/lwgeom_pg.c | 82 +++++++++++++++++++++++++++- libpgcommon/lwgeom_pg.h | 7 +++ postgis/lwgeom_backend_api.c | 84 +---------------------------- raster/rt_pg/rtpostgis.c | 102 ++++++++++++++++++++++------------- 4 files changed, 155 insertions(+), 120 deletions(-) diff --git a/libpgcommon/lwgeom_pg.c b/libpgcommon/lwgeom_pg.c index 14eac85d9..a8225adb0 100644 --- a/libpgcommon/lwgeom_pg.c +++ b/libpgcommon/lwgeom_pg.c @@ -16,8 +16,10 @@ #include #include -#include #include +#include +#include +#include #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 index d5a4d173e..b51e9084c 100644 --- 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 index cd10040ca..cafe39085 100644 --- 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 index 3dfac6902..ed36fbe2c 100644 --- 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); -- 2.50.0