From 087d8427e3791fda02a7b46e8603c09c6a5b171e Mon Sep 17 00:00:00 2001 From: Burak Yucesoy Date: Thu, 9 Mar 2017 16:11:27 +0300 Subject: [PATCH 1/8] Error out if binary citus version does not match installed extension With this change, we start to error out if loaded citus binaries does not match the available major version or installed citus extension version. In this case we force user to restart the server or run ALTER EXTENSION depending on the situation --- Makefile | 4 +- Makefile.global.in | 4 +- configure | 58 ++++- configure.in | 23 +- .../distributed/executor/multi_utility.c | 106 ++++++++ src/backend/distributed/shared_library_init.c | 13 + .../distributed/utils/metadata_cache.c | 244 +++++++++++++++++- src/include/citus_config.h.in | 12 + src/include/citus_version.h.in | 13 + src/include/distributed/metadata_cache.h | 3 + src/test/regress/expected/multi_extension.out | 62 +---- src/test/regress/sql/multi_extension.sql | 58 +---- 12 files changed, 478 insertions(+), 122 deletions(-) create mode 100644 src/include/citus_version.h.in diff --git a/Makefile b/Makefile index 10841d743..9574600ad 100644 --- a/Makefile +++ b/Makefile @@ -13,14 +13,14 @@ include Makefile.global all: extension # build extension -extension: +extension: $(citus_abs_srcdir)/src/include/citus_version.h $(MAKE) -C src/backend/distributed/ all install-extension: extension $(MAKE) -C src/backend/distributed/ install install-headers: extension $(MKDIR_P) '$(DESTDIR)$(includedir_server)/distributed/' # generated headers are located in the build directory - $(INSTALL_DATA) src/include/citus_config.h '$(DESTDIR)$(includedir_server)/' + $(INSTALL_DATA) $(citus_abs_srcdir)/src/include/citus_config.h '$(DESTDIR)$(includedir_server)/' # the rest in the source tree $(INSTALL_DATA) $(citus_abs_srcdir)/src/include/distributed/*.h '$(DESTDIR)$(includedir_server)/distributed/' clean-extension: diff --git a/Makefile.global.in b/Makefile.global.in index 7cdbcd5d9..57678e137 100644 --- a/Makefile.global.in +++ b/Makefile.global.in @@ -44,8 +44,8 @@ $(citus_top_builddir)/Makefile.global: $(citus_abs_top_srcdir)/configure $(citus # Ensure configuration is generated by the most recent configure, # useful for longer existing build directories. -$(citus_top_builddir)/config.status: $(citus_abs_top_srcdir)/configure - cd @abs_top_builddir@ && ./config.status --recheck +$(citus_top_builddir)/config.status: $(citus_abs_top_srcdir)/configure $(citus_abs_top_srcdir)/src/backend/distributed/citus.control + cd @abs_top_builddir@ && ./config.status --recheck && ./config.status # Regenerate configure if configure.in changed $(citus_abs_top_srcdir)/configure: $(citus_abs_top_srcdir)/configure.in diff --git a/configure b/configure index c46b262b8..8ec14379c 100755 --- a/configure +++ b/configure @@ -1,6 +1,6 @@ #! /bin/sh # Guess values for system-dependent variables and create Makefiles. -# Generated by GNU Autoconf 2.69 for Citus 5.0. +# Generated by GNU Autoconf 2.69 for Citus 6.2devel. # # # Copyright (C) 1992-1996, 1998-2012 Free Software Foundation, Inc. @@ -9,7 +9,7 @@ # This configure script is free software; the Free Software Foundation # gives unlimited permission to copy, distribute and modify it. # -# Copyright (c) 2012-2016, Citus Data, Inc. +# Copyright (c) 2012-2017, Citus Data, Inc. ## -------------------- ## ## M4sh Initialization. ## ## -------------------- ## @@ -579,8 +579,8 @@ MAKEFLAGS= # Identity of this package. PACKAGE_NAME='Citus' PACKAGE_TARNAME='citus' -PACKAGE_VERSION='5.0' -PACKAGE_STRING='Citus 5.0' +PACKAGE_VERSION='6.2devel' +PACKAGE_STRING='Citus 6.2devel' PACKAGE_BUGREPORT='' PACKAGE_URL='' @@ -1194,7 +1194,7 @@ if test "$ac_init_help" = "long"; then # Omit some internal or obsolete options to make the list less imposing. # This message is too long to be a string in the A/UX 3.1 sh. cat <<_ACEOF -\`configure' configures Citus 5.0 to adapt to many kinds of systems. +\`configure' configures Citus 6.2devel to adapt to many kinds of systems. Usage: $0 [OPTION]... [VAR=VALUE]... @@ -1255,7 +1255,7 @@ fi if test -n "$ac_init_help"; then case $ac_init_help in - short | recursive ) echo "Configuration of Citus 5.0:";; + short | recursive ) echo "Configuration of Citus 6.2devel:";; esac cat <<\_ACEOF @@ -1343,14 +1343,14 @@ fi test -n "$ac_init_help" && exit $ac_status if $ac_init_version; then cat <<\_ACEOF -Citus configure 5.0 +Citus configure 6.2devel generated by GNU Autoconf 2.69 Copyright (C) 2012 Free Software Foundation, Inc. This configure script is free software; the Free Software Foundation gives unlimited permission to copy, distribute and modify it. -Copyright (c) 2012-2016, Citus Data, Inc. +Copyright (c) 2012-2017, Citus Data, Inc. _ACEOF exit fi @@ -1400,7 +1400,7 @@ cat >config.log <<_ACEOF This file contains any messages produced by compilers while running configure, to aid debugging if configure makes a mistake. -It was created by Citus $as_me 5.0, which was +It was created by Citus $as_me 6.2devel, which was generated by GNU Autoconf 2.69. Invocation command line was $ $0 $@ @@ -1750,6 +1750,39 @@ ac_compiler_gnu=$ac_cv_c_compiler_gnu +# CITUS_MAJORVERSION definition +CITUS_MAJORVERSION=`expr "$PACKAGE_VERSION" : '\([0-9][0-9]*\.[0-9][0-9]*\)'` + +cat >>confdefs.h <<_ACEOF +#define CITUS_MAJORVERSION "$CITUS_MAJORVERSION" +_ACEOF + + +# CITUS_VERSION definition + +cat >>confdefs.h <<_ACEOF +#define CITUS_VERSION "$PACKAGE_VERSION" +_ACEOF + + +# CITUS_VERSION_NUM definition +CITUS_VERSION_NUM="`echo "$PACKAGE_VERSION" | sed 's/[A-Za-z].*$//' | +tr '.' ' ' | +$AWK '{printf "%d%02d%02d", $1, $2, (NF >= 3) ? $3 : 0}'`" + +cat >>confdefs.h <<_ACEOF +#define CITUS_VERSION_NUM $CITUS_VERSION_NUM +_ACEOF + + +# CITUS_EXTENSIONVERSION definition +citus_extensionversion=$(grep "^default_version" src/backend/distributed/citus.control | cut -d\' -f2) + +cat >>confdefs.h <<_ACEOF +#define CITUS_EXTENSIONVERSION "$citus_extensionversion" +_ACEOF + + { $as_echo "$as_me:${as_lineno-$LINENO}: checking for a sed that does not truncate output" >&5 $as_echo_n "checking for a sed that does not truncate output... " >&6; } if ${ac_cv_path_SED+:} false; then : @@ -2974,7 +3007,7 @@ POSTGRES_BUILDDIR="$POSTGRES_BUILDDIR" ac_config_files="$ac_config_files Makefile.global" -ac_config_headers="$ac_config_headers src/include/citus_config.h" +ac_config_headers="$ac_config_headers src/include/citus_config.h src/include/citus_version.h" cat >confcache <<\_ACEOF @@ -3483,7 +3516,7 @@ cat >>$CONFIG_STATUS <<\_ACEOF || ac_write_fail=1 # report actual input values of CONFIG_FILES etc. instead of their # values after options handling. ac_log=" -This file was extended by Citus $as_me 5.0, which was +This file was extended by Citus $as_me 6.2devel, which was generated by GNU Autoconf 2.69. Invocation command line was CONFIG_FILES = $CONFIG_FILES @@ -3545,7 +3578,7 @@ _ACEOF cat >>$CONFIG_STATUS <<_ACEOF || ac_write_fail=1 ac_cs_config="`$as_echo "$ac_configure_args" | sed 's/^ //; s/[\\""\`\$]/\\\\&/g'`" ac_cs_version="\\ -Citus config.status 5.0 +Citus config.status 6.2devel configured by $0, generated by GNU Autoconf 2.69, with options \\"\$ac_cs_config\\" @@ -3668,6 +3701,7 @@ do case $ac_config_target in "Makefile.global") CONFIG_FILES="$CONFIG_FILES Makefile.global" ;; "src/include/citus_config.h") CONFIG_HEADERS="$CONFIG_HEADERS src/include/citus_config.h" ;; + "src/include/citus_version.h") CONFIG_HEADERS="$CONFIG_HEADERS src/include/citus_version.h" ;; *) as_fn_error $? "invalid argument: \`$ac_config_target'" "$LINENO" 5;; esac diff --git a/configure.in b/configure.in index 25bcbe666..fb261d08b 100644 --- a/configure.in +++ b/configure.in @@ -5,8 +5,25 @@ # everyone needing autoconf installed, the resulting files are checked # into the SCM. -AC_INIT([Citus], [5.0], [], [citus], []) -AC_COPYRIGHT([Copyright (c) 2012-2016, Citus Data, Inc.]) +AC_INIT([Citus], [6.2devel]) +AC_COPYRIGHT([Copyright (c) 2012-2017, Citus Data, Inc.]) + +# CITUS_MAJORVERSION definition +[CITUS_MAJORVERSION=`expr "$PACKAGE_VERSION" : '\([0-9][0-9]*\.[0-9][0-9]*\)'`] +AC_DEFINE_UNQUOTED(CITUS_MAJORVERSION, "$CITUS_MAJORVERSION", [Citus major version as a string]) + +# CITUS_VERSION definition +AC_DEFINE_UNQUOTED(CITUS_VERSION, "$PACKAGE_VERSION", [Citus version as a string]) + +# CITUS_VERSION_NUM definition +[CITUS_VERSION_NUM="`echo "$PACKAGE_VERSION" | sed 's/[A-Za-z].*$//' | +tr '.' ' ' | +$AWK '{printf "%d%02d%02d", $1, $2, (NF >= 3) ? $3 : 0}'`"] +AC_DEFINE_UNQUOTED(CITUS_VERSION_NUM, $CITUS_VERSION_NUM, [Citus version as a number]) + +# CITUS_EXTENSIONVERSION definition +citus_extensionversion=$(grep "^default_version" src/backend/distributed/citus.control | cut -d\' -f2) +AC_DEFINE_UNQUOTED([CITUS_EXTENSIONVERSION], "$citus_extensionversion", [so the shared-library knows its own version]) AC_PROG_SED @@ -122,7 +139,7 @@ AC_SUBST(POSTGRES_SRCDIR, "$POSTGRES_SRCDIR") AC_SUBST(POSTGRES_BUILDDIR, "$POSTGRES_BUILDDIR") AC_CONFIG_FILES([Makefile.global]) -AC_CONFIG_HEADERS([src/include/citus_config.h]) +AC_CONFIG_HEADERS([src/include/citus_config.h] [src/include/citus_version.h]) AH_TOP([ /* * citus_config.h.in is generated by autoconf/autoheader and diff --git a/src/backend/distributed/executor/multi_utility.c b/src/backend/distributed/executor/multi_utility.c index d70fd1930..2c40167db 100644 --- a/src/backend/distributed/executor/multi_utility.c +++ b/src/backend/distributed/executor/multi_utility.c @@ -28,6 +28,7 @@ #include "catalog/namespace.h" #include "catalog/pg_attribute.h" #include "catalog/pg_class.h" +#include "citus_version.h" #include "commands/defrem.h" #include "commands/tablecmds.h" #include "commands/prepare.h" @@ -120,6 +121,7 @@ static char * DeparseVacuumColumnNames(List *columnNameList); /* Local functions forward declarations for unsupported command checks */ +static void ErrorIfUnstableCreateOrAlterExtensionStmt(Node *parsetree); static void ErrorIfUnsupportedIndexStmt(IndexStmt *createIndexStatement); static void ErrorIfUnsupportedDropIndexStmt(DropStmt *dropIndexStatement); static void ErrorIfUnsupportedAlterTableStmt(AlterTableStmt *alterTableStatement); @@ -170,6 +172,7 @@ multi_ProcessUtility(Node *parsetree, Oid savedUserId = InvalidOid; int savedSecurityContext = 0; List *ddlJobs = NIL; + bool extensionStatement = false; if (IsA(parsetree, TransactionStmt)) { @@ -185,6 +188,53 @@ multi_ProcessUtility(Node *parsetree, return; } + if (IsA(parsetree, CreateExtensionStmt)) + { + CreateExtensionStmt *createExtensionStmt = (CreateExtensionStmt *) parsetree; + if (strcmp(createExtensionStmt->extname, "citus") == 0) + { + ErrorIfUnstableCreateOrAlterExtensionStmt(parsetree); + } + + extensionStatement = true; + } + + if (IsA(parsetree, AlterExtensionStmt)) + { + AlterExtensionStmt *alterExtensionStmt = (AlterExtensionStmt *) parsetree; + if (strcmp(alterExtensionStmt->extname, "citus") == 0) + { + ErrorIfUnstableCreateOrAlterExtensionStmt(parsetree); + } + + extensionStatement = true; + } + + if (IsA(parsetree, DropStmt)) + { + DropStmt *dropStatement = (DropStmt *) parsetree; + + if (dropStatement->removeType == OBJECT_EXTENSION) + { + extensionStatement = true; + } + } + + if (extensionStatement) + { + /* + * In CitusHasBeenLoaded check below, we compare binary Citus version, + * extension version and available version. If they are different, we + * force user to execute ALTER EXTENSION citus UPDATE. Therefore, if + * user executes ALTER EXTENSION or DROP EXTENSION query we should drop + * to the standart utility before CitusHasBeenLoaded check. + */ + standard_ProcessUtility(parsetree, queryString, context, + params, dest, completionTag); + + return; + } + if (!CitusHasBeenLoaded()) { /* @@ -1256,6 +1306,62 @@ DeparseVacuumColumnNames(List *columnNameList) } +/* + * ErrorIfUnstableCreateOrAlterExtensionStmt compares CITUS_EXTENSIONVERSION + * and version given CREATE/ALTER EXTENSION statement will create/update to. If + * they are not same in major or minor version numbers, this function errors + * out. It ignores the catalog version. + */ +static void +ErrorIfUnstableCreateOrAlterExtensionStmt(Node *parsetree) +{ + List *optionsList = NIL; + ListCell *optionsCell = NULL; + + if (IsA(parsetree, CreateExtensionStmt)) + { + CreateExtensionStmt *createExtensionStmt = (CreateExtensionStmt *) parsetree; + optionsList = createExtensionStmt->options; + } + else if (IsA(parsetree, AlterExtensionStmt)) + { + AlterExtensionStmt *alterExtensionStmt = (AlterExtensionStmt *) parsetree; + optionsList = alterExtensionStmt->options; + } + else + { + ereport(ERROR, (errmsg("unsupported node type, create or alter extension " + "is expected"))); + } + + foreach(optionsCell, optionsList) + { + DefElem *defElement = (DefElem *) lfirst(optionsCell); + + if (strcmp(defElement->defname, "new_version") == 0) + { + char *newVersion = strVal(defElement->arg); + + if (!CompareVersions(newVersion, CITUS_EXTENSIONVERSION)) + { + ereport(ERROR, (errmsg("requested version is not compatible with " + "loaded Citus binaries."))); + } + + return; + } + } + + /* + * new_version is not specified in ALTER EXTENSION statement, PostgreSQL will use + * default_version from citus.control file. We will flush availableMajorVersion to + * re-check the available version from citus.control file. + */ + availableMajorVersion = NULL; + ErrorIfNewMajorVersionAvailable(); +} + + /* * ErrorIfUnsupportedIndexStmt checks if the corresponding index statement is * supported for distributed tables and errors out if it is not. diff --git a/src/backend/distributed/shared_library_init.c b/src/backend/distributed/shared_library_init.c index 689296b08..de5ee793a 100644 --- a/src/backend/distributed/shared_library_init.c +++ b/src/backend/distributed/shared_library_init.c @@ -16,6 +16,7 @@ #include "fmgr.h" #include "miscadmin.h" +#include "citus_version.h" #include "commands/explain.h" #include "executor/executor.h" #include "distributed/citus_nodefuncs.h" @@ -48,6 +49,8 @@ /* marks shared object as one loadable by the postgres version compiled against */ PG_MODULE_MAGIC; +static char *CitusVersion; + void _PG_init(void); static void CreateRequiredDirectories(void); @@ -610,6 +613,16 @@ RegisterCitusConfigVariables(void) 0, NULL, NULL, NULL); + DefineCustomStringVariable( + "citus.version", + gettext_noop("Shows the running Citus version"), + NULL, + &CitusVersion, + CITUS_VERSION, + PGC_INTERNAL, + 0, + NULL, NULL, NULL); + /* warn about config items in the citus namespace that are not registered above */ EmitWarningsOnPlaceholders("citus"); } diff --git a/src/backend/distributed/utils/metadata_cache.c b/src/backend/distributed/utils/metadata_cache.c index 2939ade41..f8360556e 100644 --- a/src/backend/distributed/utils/metadata_cache.c +++ b/src/backend/distributed/utils/metadata_cache.c @@ -22,6 +22,7 @@ #include "catalog/pg_extension.h" #include "catalog/pg_namespace.h" #include "catalog/pg_type.h" +#include "citus_version.h" #include "commands/extension.h" #include "commands/trigger.h" #include "distributed/colocation_utils.h" @@ -35,6 +36,7 @@ #include "distributed/shardinterval_utils.h" #include "distributed/worker_manager.h" #include "distributed/worker_protocol.h" +#include "executor/executor.h" #include "parser/parse_func.h" #include "utils/builtins.h" #include "utils/catcache.h" @@ -98,6 +100,10 @@ static Oid distTransactionRelationId = InvalidOid; static Oid distTransactionGroupIndexId = InvalidOid; static Oid extraDataContainerFuncId = InvalidOid; +/* Citus installed extension version */ +char *availableMajorVersion = NULL; +static char *installedExtensionVersion = NULL; + /* Hash table for informations about each partition */ static HTAB *DistTableCacheHash = NULL; @@ -133,6 +139,9 @@ static bool HasUniformHashDistribution(ShardInterval **shardIntervalArray, int shardIntervalArrayLength); static bool HasUninitializedShardInterval(ShardInterval **sortedShardIntervalArray, int shardCount); +static void ErrorIfExtensionUpdateNeeded(void); +static char * AvailableMajorVersion(void); +static char * InstalledExtensionVersion(void); static void InitializeDistTableCache(void); static void InitializeWorkerNodeCache(void); static uint32 WorkerNodeHashCode(const void *key, Size keySize); @@ -927,7 +936,7 @@ bool CitusHasBeenLoaded(void) { /* recheck presence until citus has been loaded */ - if (!extensionLoaded) + if (!extensionLoaded || creating_extension) { bool extensionPresent = false; bool extensionScriptExecuted = true; @@ -964,13 +973,246 @@ CitusHasBeenLoaded(void) * present during early stages of upgrade operation. */ DistPartitionRelationId(); + + /* + * We also set installedExtensionVersion to NULL so that it will be re-read + * in case of extension update. + */ + installedExtensionVersion = NULL; } } + if (extensionLoaded) + { + ErrorIfNewMajorVersionAvailable(); + ErrorIfExtensionUpdateNeeded(); + } + return extensionLoaded; } +/* + * ErrorIfNewMajorVersionAvailable compares CITUS_EXTENSIONVERSION and currently + * available version from citus.control file. If they are not same in major or + * minor version numbers, this function errors out. It ignores the schema version. + */ +void +ErrorIfNewMajorVersionAvailable(void) +{ + char *availableVersion = AvailableMajorVersion(); + + if (!CompareVersions(availableVersion, CITUS_EXTENSIONVERSION)) + { + ereport(ERROR, (errmsg("server restart is needed because, loaded Citus binaries " + "does not match the available extension version"))); + } +} + + +/* + * ErrorIfExtensionUpdateNeeded compares CITUS_EXTENSIONVERSION and currently and + * catalog version from pg_extemsion catalog table. If they are not same in major or + * minor version numbers, this function errors out. It ignores the schema version. + */ +static void +ErrorIfExtensionUpdateNeeded(void) +{ + char *installedVersion = InstalledExtensionVersion(); + + if (!CompareVersions(installedVersion, CITUS_EXTENSIONVERSION)) + { + ereport(ERROR, (errmsg("\"ALTER EXTENSION citus UPDATE;\" is needed, because " + "loaded Citus binaries does not match the installed " + "extension version"))); + } +} + + +/* + * CompareVersions compares given two versions. If they are same in major and + * minor version numbers, this function returns true. It ignores the schema version. + */ +bool +CompareVersions(char *leftVersion, char *rightVersion) +{ + const char schemaVersionSeparator = '-'; + char *seperatorPosition = strchr(leftVersion, schemaVersionSeparator); + int comparisionLimit = 0; + + if (seperatorPosition != NULL) + { + comparisionLimit = seperatorPosition - leftVersion; + } + else + { + comparisionLimit = strlen(leftVersion); + } + + return strncmp(leftVersion, rightVersion, comparisionLimit) == 0; +} + + +/* + * AvailableMajorVersion returns the Citus version from citus.control file. It also + * saves the result, thus consecutive calls to CitusExtensionAvailableVersion will + * not read the citus.control file again. + */ +static char * +AvailableMajorVersion(void) +{ + ReturnSetInfo *extensionsResultSet = NULL; + TupleTableSlot *tupleTableSlot = NULL; + FunctionCallInfoData *fcinfo = NULL; + FmgrInfo *flinfo = NULL; + int argumentCount = 0; + EState *estate = NULL; + + bool hasTuple = false; + bool goForward = true; + bool doCopy = false; + + /* if we cached the result before, return it*/ + if (availableMajorVersion != NULL) + { + return availableMajorVersion; + } + + estate = CreateExecutorState(); + extensionsResultSet = makeNode(ReturnSetInfo); + extensionsResultSet->econtext = GetPerTupleExprContext(estate); + extensionsResultSet->allowedModes = SFRM_Materialize; + + fcinfo = palloc0(sizeof(FunctionCallInfoData)); + flinfo = palloc0(sizeof(FmgrInfo)); + + fmgr_info(F_PG_AVAILABLE_EXTENSIONS, flinfo); + InitFunctionCallInfoData(*fcinfo, flinfo, argumentCount, InvalidOid, NULL, + (Node *) extensionsResultSet); + + /* pg_available_extensions returns result set containing all available extensions */ + (*pg_available_extensions)(fcinfo); + + tupleTableSlot = MakeSingleTupleTableSlot(extensionsResultSet->setDesc); + hasTuple = tuplestore_gettupleslot(extensionsResultSet->setResult, goForward, doCopy, + tupleTableSlot); + while (hasTuple) + { + Datum extensionNameDatum = 0; + char *extensionName = NULL; + bool isNull = false; + + extensionNameDatum = slot_getattr(tupleTableSlot, 1, &isNull); + extensionName = NameStr(*DatumGetName(extensionNameDatum)); + if (strcmp(extensionName, "citus") == 0) + { + MemoryContext oldMemoryContext = NULL; + Datum citusVersionDatum = slot_getattr(tupleTableSlot, 2, &isNull); + + /* we will cache the result of citus version to prevent catalog access */ + if (CacheMemoryContext == NULL) + { + CreateCacheMemoryContext(); + } + oldMemoryContext = MemoryContextSwitchTo(CacheMemoryContext); + + availableMajorVersion = text_to_cstring(DatumGetTextPP(citusVersionDatum)); + + MemoryContextSwitchTo(oldMemoryContext); + + ExecClearTuple(tupleTableSlot); + ExecDropSingleTupleTableSlot(tupleTableSlot); + + return availableMajorVersion; + } + + ExecClearTuple(tupleTableSlot); + hasTuple = tuplestore_gettupleslot(extensionsResultSet->setResult, goForward, + doCopy, tupleTableSlot); + } + + ExecDropSingleTupleTableSlot(tupleTableSlot); + + ereport(ERROR, (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), + errmsg("citus extension is not found"))); + + return NULL; +} + + +/* + * InstalledExtensionVersion returns the Citus version in PostgreSQL pg_extension table. + * It also saves the result, thus consecutive calls to CitusExtensionCatalogVersion + * will not read the catalog tables again. + */ +static char * +InstalledExtensionVersion(void) +{ + Relation relation = NULL; + SysScanDesc scandesc; + ScanKeyData entry[1]; + HeapTuple extensionTuple = NULL; + + /* if we cached the result before, return it*/ + if (installedExtensionVersion != NULL) + { + return installedExtensionVersion; + } + + relation = heap_open(ExtensionRelationId, AccessShareLock); + + ScanKeyInit(&entry[0], Anum_pg_extension_extname, BTEqualStrategyNumber, F_NAMEEQ, + CStringGetDatum("citus")); + + scandesc = systable_beginscan(relation, ExtensionNameIndexId, true, + NULL, 1, entry); + + extensionTuple = systable_getnext(scandesc); + + /* We assume that there can be at most one matching tuple */ + if (HeapTupleIsValid(extensionTuple)) + { + MemoryContext oldMemoryContext = NULL; + int extensionIndex = Anum_pg_extension_extversion; + TupleDesc tupleDescriptor = RelationGetDescr(relation); + bool isNull = false; + + Datum extensionVersionDatum = heap_getattr(extensionTuple, extensionIndex, + tupleDescriptor, &isNull); + + if (isNull) + { + ereport(ERROR, (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), + errmsg("citus extension version is null"))); + } + + /* we will cache the result of citus version to prevent catalog access */ + if (CacheMemoryContext == NULL) + { + CreateCacheMemoryContext(); + } + + oldMemoryContext = MemoryContextSwitchTo(CacheMemoryContext); + + installedExtensionVersion = text_to_cstring(DatumGetTextPP( + extensionVersionDatum)); + + MemoryContextSwitchTo(oldMemoryContext); + } + else + { + ereport(ERROR, (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), + errmsg("citus extension is not loaded"))); + } + + systable_endscan(scandesc); + + heap_close(relation, AccessShareLock); + + return installedExtensionVersion; +} + + /* return oid of pg_dist_shard relation */ Oid DistShardRelationId(void) diff --git a/src/include/citus_config.h.in b/src/include/citus_config.h.in index 33da446e7..97ad673aa 100644 --- a/src/include/citus_config.h.in +++ b/src/include/citus_config.h.in @@ -10,6 +10,18 @@ */ +/* so the shared-library knows its own version */ +#undef CITUS_EXTENSIONVERSION + +/* Citus major version as a string */ +#undef CITUS_MAJORVERSION + +/* Citus version as a string */ +#undef CITUS_VERSION + +/* Citus version as a number */ +#undef CITUS_VERSION_NUM + /* Define to the address where bug reports for this package should be sent. */ #undef PACKAGE_BUGREPORT diff --git a/src/include/citus_version.h.in b/src/include/citus_version.h.in new file mode 100644 index 000000000..b576c29aa --- /dev/null +++ b/src/include/citus_version.h.in @@ -0,0 +1,13 @@ +/* This file is created manually */ + +/* so the shared-library knows its own version */ +#undef CITUS_EXTENSIONVERSION + +/* Citus major version as a string */ +#undef CITUS_MAJORVERSION + +/* Citus version as a string */ +#undef CITUS_VERSION + +/* Citus version as a number */ +#undef CITUS_VERSION_NUM diff --git a/src/include/distributed/metadata_cache.h b/src/include/distributed/metadata_cache.h index abd1861c0..36b9ccfc4 100644 --- a/src/include/distributed/metadata_cache.h +++ b/src/include/distributed/metadata_cache.h @@ -17,6 +17,7 @@ #include "distributed/worker_manager.h" #include "utils/hsearch.h" +extern char *availableMajorVersion; /* * Representation of a table's metadata that is frequently used for @@ -68,6 +69,8 @@ extern void CitusInvalidateRelcacheByRelid(Oid relationId); extern void CitusInvalidateRelcacheByShardId(int64 shardId); extern bool CitusHasBeenLoaded(void); +void ErrorIfNewMajorVersionAvailable(void); +bool CompareVersions(char *leftVersion, char *rightVersion); /* access WorkerNodeHash */ extern HTAB * GetWorkerNodeHash(void); diff --git a/src/test/regress/expected/multi_extension.out b/src/test/regress/expected/multi_extension.out index 8655907f8..9b2c85c93 100644 --- a/src/test/regress/expected/multi_extension.out +++ b/src/test/regress/expected/multi_extension.out @@ -24,59 +24,19 @@ WHERE pgd.refclassid = 'pg_extension'::regclass AND -- DROP EXTENSION pre-created by the regression suite DROP EXTENSION citus; \c --- Create extension in oldest version, test every upgrade step +-- Create extension in oldest version, this is expected to fail CREATE EXTENSION citus VERSION '5.0'; -ALTER EXTENSION citus UPDATE TO '5.0-1'; -ALTER EXTENSION citus UPDATE TO '5.0-2'; -ALTER EXTENSION citus UPDATE TO '5.1-1'; -ALTER EXTENSION citus UPDATE TO '5.1-2'; -ALTER EXTENSION citus UPDATE TO '5.1-3'; -ALTER EXTENSION citus UPDATE TO '5.1-4'; -ALTER EXTENSION citus UPDATE TO '5.1-5'; -ALTER EXTENSION citus UPDATE TO '5.1-6'; -ALTER EXTENSION citus UPDATE TO '5.1-7'; -ALTER EXTENSION citus UPDATE TO '5.1-8'; -ALTER EXTENSION citus UPDATE TO '5.2-1'; -ALTER EXTENSION citus UPDATE TO '5.2-2'; -ALTER EXTENSION citus UPDATE TO '5.2-3'; -ALTER EXTENSION citus UPDATE TO '5.2-4'; -ALTER EXTENSION citus UPDATE TO '6.0-1'; -ALTER EXTENSION citus UPDATE TO '6.0-2'; -ALTER EXTENSION citus UPDATE TO '6.0-3'; -ALTER EXTENSION citus UPDATE TO '6.0-4'; -ALTER EXTENSION citus UPDATE TO '6.0-5'; -ALTER EXTENSION citus UPDATE TO '6.0-6'; -ALTER EXTENSION citus UPDATE TO '6.0-7'; -ALTER EXTENSION citus UPDATE TO '6.0-8'; -ALTER EXTENSION citus UPDATE TO '6.0-9'; -ALTER EXTENSION citus UPDATE TO '6.0-10'; -ALTER EXTENSION citus UPDATE TO '6.0-11'; -ALTER EXTENSION citus UPDATE TO '6.0-12'; -ALTER EXTENSION citus UPDATE TO '6.0-13'; -ALTER EXTENSION citus UPDATE TO '6.0-14'; -ALTER EXTENSION citus UPDATE TO '6.0-15'; -ALTER EXTENSION citus UPDATE TO '6.0-16'; -ALTER EXTENSION citus UPDATE TO '6.0-17'; -ALTER EXTENSION citus UPDATE TO '6.0-18'; -ALTER EXTENSION citus UPDATE TO '6.1-1'; -ALTER EXTENSION citus UPDATE TO '6.1-2'; -ALTER EXTENSION citus UPDATE TO '6.1-3'; -ALTER EXTENSION citus UPDATE TO '6.1-4'; -ALTER EXTENSION citus UPDATE TO '6.1-5'; -ALTER EXTENSION citus UPDATE TO '6.1-6'; -ALTER EXTENSION citus UPDATE TO '6.1-7'; -ALTER EXTENSION citus UPDATE TO '6.1-8'; -ALTER EXTENSION citus UPDATE TO '6.1-9'; -ALTER EXTENSION citus UPDATE TO '6.1-10'; -ALTER EXTENSION citus UPDATE TO '6.1-11'; -ALTER EXTENSION citus UPDATE TO '6.1-12'; -ALTER EXTENSION citus UPDATE TO '6.1-13'; -ALTER EXTENSION citus UPDATE TO '6.1-14'; -ALTER EXTENSION citus UPDATE TO '6.1-15'; -ALTER EXTENSION citus UPDATE TO '6.1-16'; -ALTER EXTENSION citus UPDATE TO '6.1-17'; -ALTER EXTENSION citus UPDATE TO '6.2-1'; +ERROR: requested version is not compatible with loaded Citus binaries. +-- Create extension in newest major version, test every upgrade step +CREATE EXTENSION citus VERSION '6.2-1'; ALTER EXTENSION citus UPDATE TO '6.2-2'; +-- show running version +SHOW citus.version; + citus.version +--------------- + 6.2devel +(1 row) + -- ensure no objects were created outside pg_catalog SELECT COUNT(*) FROM pg_depend AS pgd, diff --git a/src/test/regress/sql/multi_extension.sql b/src/test/regress/sql/multi_extension.sql index d1a1ff559..1b4d13f97 100644 --- a/src/test/regress/sql/multi_extension.sql +++ b/src/test/regress/sql/multi_extension.sql @@ -24,60 +24,16 @@ WHERE pgd.refclassid = 'pg_extension'::regclass AND DROP EXTENSION citus; \c --- Create extension in oldest version, test every upgrade step +-- Create extension in oldest version, this is expected to fail CREATE EXTENSION citus VERSION '5.0'; -ALTER EXTENSION citus UPDATE TO '5.0-1'; -ALTER EXTENSION citus UPDATE TO '5.0-2'; -ALTER EXTENSION citus UPDATE TO '5.1-1'; -ALTER EXTENSION citus UPDATE TO '5.1-2'; -ALTER EXTENSION citus UPDATE TO '5.1-3'; -ALTER EXTENSION citus UPDATE TO '5.1-4'; -ALTER EXTENSION citus UPDATE TO '5.1-5'; -ALTER EXTENSION citus UPDATE TO '5.1-6'; -ALTER EXTENSION citus UPDATE TO '5.1-7'; -ALTER EXTENSION citus UPDATE TO '5.1-8'; -ALTER EXTENSION citus UPDATE TO '5.2-1'; -ALTER EXTENSION citus UPDATE TO '5.2-2'; -ALTER EXTENSION citus UPDATE TO '5.2-3'; -ALTER EXTENSION citus UPDATE TO '5.2-4'; -ALTER EXTENSION citus UPDATE TO '6.0-1'; -ALTER EXTENSION citus UPDATE TO '6.0-2'; -ALTER EXTENSION citus UPDATE TO '6.0-3'; -ALTER EXTENSION citus UPDATE TO '6.0-4'; -ALTER EXTENSION citus UPDATE TO '6.0-5'; -ALTER EXTENSION citus UPDATE TO '6.0-6'; -ALTER EXTENSION citus UPDATE TO '6.0-7'; -ALTER EXTENSION citus UPDATE TO '6.0-8'; -ALTER EXTENSION citus UPDATE TO '6.0-9'; -ALTER EXTENSION citus UPDATE TO '6.0-10'; -ALTER EXTENSION citus UPDATE TO '6.0-11'; -ALTER EXTENSION citus UPDATE TO '6.0-12'; -ALTER EXTENSION citus UPDATE TO '6.0-13'; -ALTER EXTENSION citus UPDATE TO '6.0-14'; -ALTER EXTENSION citus UPDATE TO '6.0-15'; -ALTER EXTENSION citus UPDATE TO '6.0-16'; -ALTER EXTENSION citus UPDATE TO '6.0-17'; -ALTER EXTENSION citus UPDATE TO '6.0-18'; -ALTER EXTENSION citus UPDATE TO '6.1-1'; -ALTER EXTENSION citus UPDATE TO '6.1-2'; -ALTER EXTENSION citus UPDATE TO '6.1-3'; -ALTER EXTENSION citus UPDATE TO '6.1-4'; -ALTER EXTENSION citus UPDATE TO '6.1-5'; -ALTER EXTENSION citus UPDATE TO '6.1-6'; -ALTER EXTENSION citus UPDATE TO '6.1-7'; -ALTER EXTENSION citus UPDATE TO '6.1-8'; -ALTER EXTENSION citus UPDATE TO '6.1-9'; -ALTER EXTENSION citus UPDATE TO '6.1-10'; -ALTER EXTENSION citus UPDATE TO '6.1-11'; -ALTER EXTENSION citus UPDATE TO '6.1-12'; -ALTER EXTENSION citus UPDATE TO '6.1-13'; -ALTER EXTENSION citus UPDATE TO '6.1-14'; -ALTER EXTENSION citus UPDATE TO '6.1-15'; -ALTER EXTENSION citus UPDATE TO '6.1-16'; -ALTER EXTENSION citus UPDATE TO '6.1-17'; -ALTER EXTENSION citus UPDATE TO '6.2-1'; + +-- Create extension in newest major version, test every upgrade step +CREATE EXTENSION citus VERSION '6.2-1'; ALTER EXTENSION citus UPDATE TO '6.2-2'; +-- show running version +SHOW citus.version; + -- ensure no objects were created outside pg_catalog SELECT COUNT(*) FROM pg_depend AS pgd, From 1c2056ec74044ff64ec2b32ff9fd6b76fee2b7fc Mon Sep 17 00:00:00 2001 From: Jason Petersen Date: Mon, 3 Apr 2017 22:18:58 -0600 Subject: [PATCH 2/8] Self-implemented review feedback The use of a bare src/ rather than $srcdir caused configure to fail during VPATH builds. With our additional dependency upon AWK, we need to call AC_PROG_AWK, otherwise environments may not have $AWK set. Finally, citus_version.h should be in .gitignore. --- configure | 112 +++++++++++++++++++++++---------- configure.in | 17 ++--- src/include/.gitignore | 2 + src/include/citus_config.h.in | 2 +- src/include/citus_version.h.in | 2 +- 5 files changed, 93 insertions(+), 42 deletions(-) diff --git a/configure b/configure index 8ec14379c..9cf33a4bd 100755 --- a/configure +++ b/configure @@ -600,6 +600,7 @@ vpath_build PATH PG_CONFIG FLEX +AWK SED target_alias host_alias @@ -1750,39 +1751,7 @@ ac_compiler_gnu=$ac_cv_c_compiler_gnu -# CITUS_MAJORVERSION definition -CITUS_MAJORVERSION=`expr "$PACKAGE_VERSION" : '\([0-9][0-9]*\.[0-9][0-9]*\)'` - -cat >>confdefs.h <<_ACEOF -#define CITUS_MAJORVERSION "$CITUS_MAJORVERSION" -_ACEOF - - -# CITUS_VERSION definition - -cat >>confdefs.h <<_ACEOF -#define CITUS_VERSION "$PACKAGE_VERSION" -_ACEOF - - -# CITUS_VERSION_NUM definition -CITUS_VERSION_NUM="`echo "$PACKAGE_VERSION" | sed 's/[A-Za-z].*$//' | -tr '.' ' ' | -$AWK '{printf "%d%02d%02d", $1, $2, (NF >= 3) ? $3 : 0}'`" - -cat >>confdefs.h <<_ACEOF -#define CITUS_VERSION_NUM $CITUS_VERSION_NUM -_ACEOF - - -# CITUS_EXTENSIONVERSION definition -citus_extensionversion=$(grep "^default_version" src/backend/distributed/citus.control | cut -d\' -f2) - -cat >>confdefs.h <<_ACEOF -#define CITUS_EXTENSIONVERSION "$citus_extensionversion" -_ACEOF - - +# we'll need sed and awk for some of the version commands { $as_echo "$as_me:${as_lineno-$LINENO}: checking for a sed that does not truncate output" >&5 $as_echo_n "checking for a sed that does not truncate output... " >&6; } if ${ac_cv_path_SED+:} false; then : @@ -1852,6 +1821,82 @@ $as_echo "$ac_cv_path_SED" >&6; } SED="$ac_cv_path_SED" rm -f conftest.sed +for ac_prog in gawk mawk nawk awk +do + # Extract the first word of "$ac_prog", so it can be a program name with args. +set dummy $ac_prog; ac_word=$2 +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for $ac_word" >&5 +$as_echo_n "checking for $ac_word... " >&6; } +if ${ac_cv_prog_AWK+:} false; then : + $as_echo_n "(cached) " >&6 +else + if test -n "$AWK"; then + ac_cv_prog_AWK="$AWK" # Let the user override the test. +else +as_save_IFS=$IFS; IFS=$PATH_SEPARATOR +for as_dir in $PATH +do + IFS=$as_save_IFS + test -z "$as_dir" && as_dir=. + for ac_exec_ext in '' $ac_executable_extensions; do + if as_fn_executable_p "$as_dir/$ac_word$ac_exec_ext"; then + ac_cv_prog_AWK="$ac_prog" + $as_echo "$as_me:${as_lineno-$LINENO}: found $as_dir/$ac_word$ac_exec_ext" >&5 + break 2 + fi +done + done +IFS=$as_save_IFS + +fi +fi +AWK=$ac_cv_prog_AWK +if test -n "$AWK"; then + { $as_echo "$as_me:${as_lineno-$LINENO}: result: $AWK" >&5 +$as_echo "$AWK" >&6; } +else + { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5 +$as_echo "no" >&6; } +fi + + + test -n "$AWK" && break +done + + +# CITUS_VERSION definition + +cat >>confdefs.h <<_ACEOF +#define CITUS_VERSION "$PACKAGE_VERSION" +_ACEOF + + +# CITUS_MAJORVERSION definition +CITUS_MAJORVERSION=`expr "$PACKAGE_VERSION" : '\([0-9][0-9]*\.[0-9][0-9]*\)'` + +cat >>confdefs.h <<_ACEOF +#define CITUS_MAJORVERSION "$CITUS_MAJORVERSION" +_ACEOF + + +# CITUS_VERSION_NUM definition +# awk -F is a regex on some platforms, and not on others, so make "." a tab +CITUS_VERSION_NUM="`echo "$PACKAGE_VERSION" | sed 's/[A-Za-z].*$//' | +tr '.' ' ' | +$AWK '{printf "%d%02d%02d", $1, $2, (NF >= 3) ? $3 : 0}'`" + +cat >>confdefs.h <<_ACEOF +#define CITUS_VERSION_NUM $CITUS_VERSION_NUM +_ACEOF + + +# CITUS_EXTENSIONVERSION definition +CITUS_EXTENSIONVERSION="`grep '^default_version' $srcdir/src/backend/distributed/citus.control | cut -d\' -f2`" + +cat >>confdefs.h <<_ACEOF +#define CITUS_EXTENSIONVERSION "$CITUS_EXTENSIONVERSION" +_ACEOF + # Re-check for flex. That allows to compile citus against a postgres # which was built without flex available (possible because generated @@ -3588,6 +3633,7 @@ gives unlimited permission to copy, distribute and modify it." ac_pwd='$ac_pwd' srcdir='$srcdir' +AWK='$AWK' test -n "\$AWK" || AWK=awk _ACEOF diff --git a/configure.in b/configure.in index fb261d08b..1bc1a6ee3 100644 --- a/configure.in +++ b/configure.in @@ -8,24 +8,27 @@ AC_INIT([Citus], [6.2devel]) AC_COPYRIGHT([Copyright (c) 2012-2017, Citus Data, Inc.]) -# CITUS_MAJORVERSION definition -[CITUS_MAJORVERSION=`expr "$PACKAGE_VERSION" : '\([0-9][0-9]*\.[0-9][0-9]*\)'`] -AC_DEFINE_UNQUOTED(CITUS_MAJORVERSION, "$CITUS_MAJORVERSION", [Citus major version as a string]) +# we'll need sed and awk for some of the version commands +AC_PROG_SED +AC_PROG_AWK # CITUS_VERSION definition AC_DEFINE_UNQUOTED(CITUS_VERSION, "$PACKAGE_VERSION", [Citus version as a string]) +# CITUS_MAJORVERSION definition +[CITUS_MAJORVERSION=`expr "$PACKAGE_VERSION" : '\([0-9][0-9]*\.[0-9][0-9]*\)'`] +AC_DEFINE_UNQUOTED(CITUS_MAJORVERSION, "$CITUS_MAJORVERSION", [Citus major version as a string]) + # CITUS_VERSION_NUM definition +# awk -F is a regex on some platforms, and not on others, so make "." a tab [CITUS_VERSION_NUM="`echo "$PACKAGE_VERSION" | sed 's/[A-Za-z].*$//' | tr '.' ' ' | $AWK '{printf "%d%02d%02d", $1, $2, (NF >= 3) ? $3 : 0}'`"] AC_DEFINE_UNQUOTED(CITUS_VERSION_NUM, $CITUS_VERSION_NUM, [Citus version as a number]) # CITUS_EXTENSIONVERSION definition -citus_extensionversion=$(grep "^default_version" src/backend/distributed/citus.control | cut -d\' -f2) -AC_DEFINE_UNQUOTED([CITUS_EXTENSIONVERSION], "$citus_extensionversion", [so the shared-library knows its own version]) - -AC_PROG_SED +[CITUS_EXTENSIONVERSION="`grep '^default_version' $srcdir/src/backend/distributed/citus.control | cut -d\' -f2`"] +AC_DEFINE_UNQUOTED([CITUS_EXTENSIONVERSION], "$CITUS_EXTENSIONVERSION", [Extension version expected by this Citus build]) # Re-check for flex. That allows to compile citus against a postgres # which was built without flex available (possible because generated diff --git a/src/include/.gitignore b/src/include/.gitignore index 5207873bf..e97125f70 100644 --- a/src/include/.gitignore +++ b/src/include/.gitignore @@ -2,3 +2,5 @@ /stamp-ext-h /citus_config.h /citus_config.h.in~ +/citus_version.h +/citus_version.h.in~ diff --git a/src/include/citus_config.h.in b/src/include/citus_config.h.in index 97ad673aa..364d22d47 100644 --- a/src/include/citus_config.h.in +++ b/src/include/citus_config.h.in @@ -10,7 +10,7 @@ */ -/* so the shared-library knows its own version */ +/* Extension version expected by this Citus build */ #undef CITUS_EXTENSIONVERSION /* Citus major version as a string */ diff --git a/src/include/citus_version.h.in b/src/include/citus_version.h.in index b576c29aa..2367b86fa 100644 --- a/src/include/citus_version.h.in +++ b/src/include/citus_version.h.in @@ -1,6 +1,6 @@ /* This file is created manually */ -/* so the shared-library knows its own version */ +/* Extension version expected by this Citus build */ #undef CITUS_EXTENSIONVERSION /* Citus major version as a string */ From a09614553fa78da015cb06d74bbcca9dfb1891b2 Mon Sep 17 00:00:00 2001 From: Burak Yucesoy Date: Tue, 4 Apr 2017 15:37:07 +0300 Subject: [PATCH 3/8] Add enable_version_checks GUC and address feedback --- Makefile | 2 +- .../distributed/executor/multi_utility.c | 31 ++--- src/backend/distributed/shared_library_init.c | 12 +- .../distributed/utils/metadata_cache.c | 110 ++++++++++++------ src/include/distributed/metadata_cache.h | 7 +- src/include/distributed/multi_utility.h | 1 + src/test/regress/expected/multi_extension.out | 62 +++++++++- src/test/regress/sql/multi_extension.sql | 63 +++++++++- 8 files changed, 222 insertions(+), 66 deletions(-) diff --git a/Makefile b/Makefile index 9574600ad..43ca841c4 100644 --- a/Makefile +++ b/Makefile @@ -20,7 +20,7 @@ install-extension: extension install-headers: extension $(MKDIR_P) '$(DESTDIR)$(includedir_server)/distributed/' # generated headers are located in the build directory - $(INSTALL_DATA) $(citus_abs_srcdir)/src/include/citus_config.h '$(DESTDIR)$(includedir_server)/' + $(INSTALL_DATA) $(citus_abs_srcdir)/src/include/citus_version.h '$(DESTDIR)$(includedir_server)/' # the rest in the source tree $(INSTALL_DATA) $(citus_abs_srcdir)/src/include/distributed/*.h '$(DESTDIR)$(includedir_server)/distributed/' clean-extension: diff --git a/src/backend/distributed/executor/multi_utility.c b/src/backend/distributed/executor/multi_utility.c index 2c40167db..fa1a3cd51 100644 --- a/src/backend/distributed/executor/multi_utility.c +++ b/src/backend/distributed/executor/multi_utility.c @@ -83,7 +83,6 @@ bool EnableDDLPropagation = true; /* ddl propagation is enabled */ - /* * This struct defines the state for the callback for drop statements. * It is copied as it is from commands/tablecmds.c in Postgres source. @@ -173,6 +172,7 @@ multi_ProcessUtility(Node *parsetree, int savedSecurityContext = 0; List *ddlJobs = NIL; bool extensionStatement = false; + bool checkExtensionVersion = false; if (IsA(parsetree, TransactionStmt)) { @@ -193,7 +193,7 @@ multi_ProcessUtility(Node *parsetree, CreateExtensionStmt *createExtensionStmt = (CreateExtensionStmt *) parsetree; if (strcmp(createExtensionStmt->extname, "citus") == 0) { - ErrorIfUnstableCreateOrAlterExtensionStmt(parsetree); + checkExtensionVersion = true; } extensionStatement = true; @@ -204,7 +204,7 @@ multi_ProcessUtility(Node *parsetree, AlterExtensionStmt *alterExtensionStmt = (AlterExtensionStmt *) parsetree; if (strcmp(alterExtensionStmt->extname, "citus") == 0) { - ErrorIfUnstableCreateOrAlterExtensionStmt(parsetree); + checkExtensionVersion = true; } extensionStatement = true; @@ -220,18 +220,23 @@ multi_ProcessUtility(Node *parsetree, } } - if (extensionStatement) + if (extensionStatement || IsA(parsetree, VariableSetStmt)) { /* * In CitusHasBeenLoaded check below, we compare binary Citus version, * extension version and available version. If they are different, we - * force user to execute ALTER EXTENSION citus UPDATE. Therefore, if - * user executes ALTER EXTENSION or DROP EXTENSION query we should drop - * to the standart utility before CitusHasBeenLoaded check. + * force user to execute ALTER EXTENSION citus UPDATE. Therefore, we + * drop to standard_ProcessUtility earlier for some commands which are + * safe and necessary to user even in version mismatch situation. */ standard_ProcessUtility(parsetree, queryString, context, params, dest, completionTag); + if (EnableVersionChecks && checkExtensionVersion) + { + ErrorIfUnstableCreateOrAlterExtensionStmt(parsetree); + } + return; } @@ -1310,7 +1315,7 @@ DeparseVacuumColumnNames(List *columnNameList) * ErrorIfUnstableCreateOrAlterExtensionStmt compares CITUS_EXTENSIONVERSION * and version given CREATE/ALTER EXTENSION statement will create/update to. If * they are not same in major or minor version numbers, this function errors - * out. It ignores the catalog version. + * out. It ignores the schema version. */ static void ErrorIfUnstableCreateOrAlterExtensionStmt(Node *parsetree) @@ -1342,7 +1347,7 @@ ErrorIfUnstableCreateOrAlterExtensionStmt(Node *parsetree) { char *newVersion = strVal(defElement->arg); - if (!CompareVersions(newVersion, CITUS_EXTENSIONVERSION)) + if (!MajorVersionsCompatible(newVersion, CITUS_EXTENSIONVERSION)) { ereport(ERROR, (errmsg("requested version is not compatible with " "loaded Citus binaries."))); @@ -1354,11 +1359,11 @@ ErrorIfUnstableCreateOrAlterExtensionStmt(Node *parsetree) /* * new_version is not specified in ALTER EXTENSION statement, PostgreSQL will use - * default_version from citus.control file. We will flush availableMajorVersion to - * re-check the available version from citus.control file. + * default_version from citus.control file. We will flush availableExtensionVersion + * to re-check the available version from citus.control file. */ - availableMajorVersion = NULL; - ErrorIfNewMajorVersionAvailable(); + availableExtensionVersion = NULL; + ErrorIfAvailableVersionMismatch(); } diff --git a/src/backend/distributed/shared_library_init.c b/src/backend/distributed/shared_library_init.c index de5ee793a..fdd428bb2 100644 --- a/src/backend/distributed/shared_library_init.c +++ b/src/backend/distributed/shared_library_init.c @@ -49,7 +49,7 @@ /* marks shared object as one loadable by the postgres version compiled against */ PG_MODULE_MAGIC; -static char *CitusVersion; +static char *CitusVersion = CITUS_VERSION; void _PG_init(void); @@ -623,6 +623,16 @@ RegisterCitusConfigVariables(void) 0, NULL, NULL, NULL); + DefineCustomBoolVariable( + "citus.enable_version_checks", + gettext_noop("Enables version checks in CREATE/ALTER extension queries"), + NULL, + &EnableVersionChecks, + true, + PGC_USERSET, + GUC_NO_SHOW_ALL, + NULL, NULL, NULL); + /* warn about config items in the citus namespace that are not registered above */ EmitWarningsOnPlaceholders("citus"); } diff --git a/src/backend/distributed/utils/metadata_cache.c b/src/backend/distributed/utils/metadata_cache.c index f8360556e..33026bb74 100644 --- a/src/backend/distributed/utils/metadata_cache.c +++ b/src/backend/distributed/utils/metadata_cache.c @@ -100,8 +100,10 @@ static Oid distTransactionRelationId = InvalidOid; static Oid distTransactionGroupIndexId = InvalidOid; static Oid extraDataContainerFuncId = InvalidOid; -/* Citus installed extension version */ -char *availableMajorVersion = NULL; +/* Citus extension version variables */ +bool EnableVersionChecks = true; /* version checks are enabled */ + +char *availableExtensionVersion = NULL; static char *installedExtensionVersion = NULL; /* Hash table for informations about each partition */ @@ -139,8 +141,8 @@ static bool HasUniformHashDistribution(ShardInterval **shardIntervalArray, int shardIntervalArrayLength); static bool HasUninitializedShardInterval(ShardInterval **sortedShardIntervalArray, int shardCount); -static void ErrorIfExtensionUpdateNeeded(void); -static char * AvailableMajorVersion(void); +static void ErrorIfInstalledVersionMismatch(void); +static char * AvailableExtensionVersion(void); static char * InstalledExtensionVersion(void); static void InitializeDistTableCache(void); static void InitializeWorkerNodeCache(void); @@ -984,8 +986,8 @@ CitusHasBeenLoaded(void) if (extensionLoaded) { - ErrorIfNewMajorVersionAvailable(); - ErrorIfExtensionUpdateNeeded(); + ErrorIfAvailableVersionMismatch(); + ErrorIfInstalledVersionMismatch(); } return extensionLoaded; @@ -993,16 +995,23 @@ CitusHasBeenLoaded(void) /* - * ErrorIfNewMajorVersionAvailable compares CITUS_EXTENSIONVERSION and currently - * available version from citus.control file. If they are not same in major or - * minor version numbers, this function errors out. It ignores the schema version. + * ErrorIfAvailableExtensionVersionMismatch compares CITUS_EXTENSIONVERSION and + * currently available version from citus.control file. If they are not same in + * major or minor version numbers, this function errors out. It ignores the schema + * version. */ void -ErrorIfNewMajorVersionAvailable(void) +ErrorIfAvailableVersionMismatch(void) { - char *availableVersion = AvailableMajorVersion(); + char *availableVersion = NULL; - if (!CompareVersions(availableVersion, CITUS_EXTENSIONVERSION)) + if (!EnableVersionChecks) + { + return; + } + + availableVersion = AvailableExtensionVersion(); + if (!MajorVersionsCompatible(availableVersion, CITUS_EXTENSIONVERSION)) { ereport(ERROR, (errmsg("server restart is needed because, loaded Citus binaries " "does not match the available extension version"))); @@ -1011,16 +1020,23 @@ ErrorIfNewMajorVersionAvailable(void) /* - * ErrorIfExtensionUpdateNeeded compares CITUS_EXTENSIONVERSION and currently and - * catalog version from pg_extemsion catalog table. If they are not same in major or - * minor version numbers, this function errors out. It ignores the schema version. + * ErrorIfInstalledVersionMismatch compares CITUS_EXTENSIONVERSION and currently + * and catalog version from pg_extemsion catalog table. If they are not same in + * major or minor version numbers, this function errors out. It ignores the schema + * version. */ static void -ErrorIfExtensionUpdateNeeded(void) +ErrorIfInstalledVersionMismatch(void) { - char *installedVersion = InstalledExtensionVersion(); + char *installedVersion = NULL; - if (!CompareVersions(installedVersion, CITUS_EXTENSIONVERSION)) + if (!EnableVersionChecks) + { + return; + } + + installedVersion = InstalledExtensionVersion(); + if (!MajorVersionsCompatible(installedVersion, CITUS_EXTENSIONVERSION)) { ereport(ERROR, (errmsg("\"ALTER EXTENSION citus UPDATE;\" is needed, because " "loaded Citus binaries does not match the installed " @@ -1030,36 +1046,55 @@ ErrorIfExtensionUpdateNeeded(void) /* - * CompareVersions compares given two versions. If they are same in major and - * minor version numbers, this function returns true. It ignores the schema version. + * MajorVersionsCompatible compares given two versions. If they are same in major + * and minor version numbers, this function returns true. It ignores the schema + * version. */ bool -CompareVersions(char *leftVersion, char *rightVersion) +MajorVersionsCompatible(char *leftVersion, char *rightVersion) { const char schemaVersionSeparator = '-'; - char *seperatorPosition = strchr(leftVersion, schemaVersionSeparator); - int comparisionLimit = 0; - if (seperatorPosition != NULL) + char *leftSeperatorPosition = strchr(leftVersion, schemaVersionSeparator); + char *rightSeperatorPosition = strchr(rightVersion, schemaVersionSeparator); + int leftComparisionLimit = 0; + int rightComparisionLimit = 0; + + if (leftSeperatorPosition != NULL) { - comparisionLimit = seperatorPosition - leftVersion; + leftComparisionLimit = leftSeperatorPosition - leftVersion; } else { - comparisionLimit = strlen(leftVersion); + leftComparisionLimit = strlen(leftVersion); } - return strncmp(leftVersion, rightVersion, comparisionLimit) == 0; + if (rightSeperatorPosition != NULL) + { + rightComparisionLimit = rightSeperatorPosition - rightVersion; + } + else + { + rightComparisionLimit = strlen(leftVersion); + } + + /* we can error out early if hypens are not in the same position */ + if (leftComparisionLimit != rightComparisionLimit) + { + return false; + } + + return strncmp(leftVersion, rightVersion, leftComparisionLimit) == 0; } /* - * AvailableMajorVersion returns the Citus version from citus.control file. It also + * AvailableExtensionVersion returns the Citus version from citus.control file. It also * saves the result, thus consecutive calls to CitusExtensionAvailableVersion will * not read the citus.control file again. */ static char * -AvailableMajorVersion(void) +AvailableExtensionVersion(void) { ReturnSetInfo *extensionsResultSet = NULL; TupleTableSlot *tupleTableSlot = NULL; @@ -1073,9 +1108,9 @@ AvailableMajorVersion(void) bool doCopy = false; /* if we cached the result before, return it*/ - if (availableMajorVersion != NULL) + if (availableExtensionVersion != NULL) { - return availableMajorVersion; + return availableExtensionVersion; } estate = CreateExecutorState(); @@ -1107,7 +1142,7 @@ AvailableMajorVersion(void) if (strcmp(extensionName, "citus") == 0) { MemoryContext oldMemoryContext = NULL; - Datum citusVersionDatum = slot_getattr(tupleTableSlot, 2, &isNull); + Datum availableVersion = slot_getattr(tupleTableSlot, 2, &isNull); /* we will cache the result of citus version to prevent catalog access */ if (CacheMemoryContext == NULL) @@ -1116,14 +1151,14 @@ AvailableMajorVersion(void) } oldMemoryContext = MemoryContextSwitchTo(CacheMemoryContext); - availableMajorVersion = text_to_cstring(DatumGetTextPP(citusVersionDatum)); + availableExtensionVersion = text_to_cstring(DatumGetTextPP(availableVersion)); MemoryContextSwitchTo(oldMemoryContext); ExecClearTuple(tupleTableSlot); ExecDropSingleTupleTableSlot(tupleTableSlot); - return availableMajorVersion; + return availableExtensionVersion; } ExecClearTuple(tupleTableSlot); @@ -1177,8 +1212,8 @@ InstalledExtensionVersion(void) TupleDesc tupleDescriptor = RelationGetDescr(relation); bool isNull = false; - Datum extensionVersionDatum = heap_getattr(extensionTuple, extensionIndex, - tupleDescriptor, &isNull); + Datum installedVersion = heap_getattr(extensionTuple, extensionIndex, + tupleDescriptor, &isNull); if (isNull) { @@ -1194,8 +1229,7 @@ InstalledExtensionVersion(void) oldMemoryContext = MemoryContextSwitchTo(CacheMemoryContext); - installedExtensionVersion = text_to_cstring(DatumGetTextPP( - extensionVersionDatum)); + installedExtensionVersion = text_to_cstring(DatumGetTextPP(installedVersion)); MemoryContextSwitchTo(oldMemoryContext); } diff --git a/src/include/distributed/metadata_cache.h b/src/include/distributed/metadata_cache.h index 36b9ccfc4..5b87a59a4 100644 --- a/src/include/distributed/metadata_cache.h +++ b/src/include/distributed/metadata_cache.h @@ -17,7 +17,8 @@ #include "distributed/worker_manager.h" #include "utils/hsearch.h" -extern char *availableMajorVersion; +extern bool EnableVersionChecks; +extern char *availableExtensionVersion; /* * Representation of a table's metadata that is frequently used for @@ -69,8 +70,8 @@ extern void CitusInvalidateRelcacheByRelid(Oid relationId); extern void CitusInvalidateRelcacheByShardId(int64 shardId); extern bool CitusHasBeenLoaded(void); -void ErrorIfNewMajorVersionAvailable(void); -bool CompareVersions(char *leftVersion, char *rightVersion); +void ErrorIfAvailableVersionMismatch(void); +bool MajorVersionsCompatible(char *leftVersion, char *rightVersion); /* access WorkerNodeHash */ extern HTAB * GetWorkerNodeHash(void); diff --git a/src/include/distributed/multi_utility.h b/src/include/distributed/multi_utility.h index 7444c10a1..84ed37443 100644 --- a/src/include/distributed/multi_utility.h +++ b/src/include/distributed/multi_utility.h @@ -13,6 +13,7 @@ #include "tcop/utility.h" extern bool EnableDDLPropagation; +extern bool EnableVersionChecks; /* * A DDLJob encapsulates the remote tasks and commands needed to process all or diff --git a/src/test/regress/expected/multi_extension.out b/src/test/regress/expected/multi_extension.out index 9b2c85c93..6448fdbda 100644 --- a/src/test/regress/expected/multi_extension.out +++ b/src/test/regress/expected/multi_extension.out @@ -24,11 +24,59 @@ WHERE pgd.refclassid = 'pg_extension'::regclass AND -- DROP EXTENSION pre-created by the regression suite DROP EXTENSION citus; \c --- Create extension in oldest version, this is expected to fail +SET citus.enable_version_checks TO 'false'; +-- Create extension in oldest version CREATE EXTENSION citus VERSION '5.0'; -ERROR: requested version is not compatible with loaded Citus binaries. --- Create extension in newest major version, test every upgrade step -CREATE EXTENSION citus VERSION '6.2-1'; +ALTER EXTENSION citus UPDATE TO '5.0-1'; +ALTER EXTENSION citus UPDATE TO '5.0-2'; +ALTER EXTENSION citus UPDATE TO '5.1-1'; +ALTER EXTENSION citus UPDATE TO '5.1-2'; +ALTER EXTENSION citus UPDATE TO '5.1-3'; +ALTER EXTENSION citus UPDATE TO '5.1-4'; +ALTER EXTENSION citus UPDATE TO '5.1-5'; +ALTER EXTENSION citus UPDATE TO '5.1-6'; +ALTER EXTENSION citus UPDATE TO '5.1-7'; +ALTER EXTENSION citus UPDATE TO '5.1-8'; +ALTER EXTENSION citus UPDATE TO '5.2-1'; +ALTER EXTENSION citus UPDATE TO '5.2-2'; +ALTER EXTENSION citus UPDATE TO '5.2-3'; +ALTER EXTENSION citus UPDATE TO '5.2-4'; +ALTER EXTENSION citus UPDATE TO '6.0-1'; +ALTER EXTENSION citus UPDATE TO '6.0-2'; +ALTER EXTENSION citus UPDATE TO '6.0-3'; +ALTER EXTENSION citus UPDATE TO '6.0-4'; +ALTER EXTENSION citus UPDATE TO '6.0-5'; +ALTER EXTENSION citus UPDATE TO '6.0-6'; +ALTER EXTENSION citus UPDATE TO '6.0-7'; +ALTER EXTENSION citus UPDATE TO '6.0-8'; +ALTER EXTENSION citus UPDATE TO '6.0-9'; +ALTER EXTENSION citus UPDATE TO '6.0-10'; +ALTER EXTENSION citus UPDATE TO '6.0-11'; +ALTER EXTENSION citus UPDATE TO '6.0-12'; +ALTER EXTENSION citus UPDATE TO '6.0-13'; +ALTER EXTENSION citus UPDATE TO '6.0-14'; +ALTER EXTENSION citus UPDATE TO '6.0-15'; +ALTER EXTENSION citus UPDATE TO '6.0-16'; +ALTER EXTENSION citus UPDATE TO '6.0-17'; +ALTER EXTENSION citus UPDATE TO '6.0-18'; +ALTER EXTENSION citus UPDATE TO '6.1-1'; +ALTER EXTENSION citus UPDATE TO '6.1-2'; +ALTER EXTENSION citus UPDATE TO '6.1-3'; +ALTER EXTENSION citus UPDATE TO '6.1-4'; +ALTER EXTENSION citus UPDATE TO '6.1-5'; +ALTER EXTENSION citus UPDATE TO '6.1-6'; +ALTER EXTENSION citus UPDATE TO '6.1-7'; +ALTER EXTENSION citus UPDATE TO '6.1-8'; +ALTER EXTENSION citus UPDATE TO '6.1-9'; +ALTER EXTENSION citus UPDATE TO '6.1-10'; +ALTER EXTENSION citus UPDATE TO '6.1-11'; +ALTER EXTENSION citus UPDATE TO '6.1-12'; +ALTER EXTENSION citus UPDATE TO '6.1-13'; +ALTER EXTENSION citus UPDATE TO '6.1-14'; +ALTER EXTENSION citus UPDATE TO '6.1-15'; +ALTER EXTENSION citus UPDATE TO '6.1-16'; +ALTER EXTENSION citus UPDATE TO '6.1-17'; +ALTER EXTENSION citus UPDATE TO '6.2-1'; ALTER EXTENSION citus UPDATE TO '6.2-2'; -- show running version SHOW citus.version; @@ -51,7 +99,11 @@ WHERE pgd.refclassid = 'pg_extension'::regclass AND 0 (1 row) --- drop extension an re-create in newest version +-- see incompatible version errors out DROP EXTENSION citus; \c +CREATE EXTENSION citus VERSION '5.0'; +ERROR: requested version is not compatible with loaded Citus binaries. +-- re-create in newest version +\c CREATE EXTENSION citus; diff --git a/src/test/regress/sql/multi_extension.sql b/src/test/regress/sql/multi_extension.sql index 1b4d13f97..3f1746c54 100644 --- a/src/test/regress/sql/multi_extension.sql +++ b/src/test/regress/sql/multi_extension.sql @@ -24,11 +24,60 @@ WHERE pgd.refclassid = 'pg_extension'::regclass AND DROP EXTENSION citus; \c --- Create extension in oldest version, this is expected to fail -CREATE EXTENSION citus VERSION '5.0'; +SET citus.enable_version_checks TO 'false'; --- Create extension in newest major version, test every upgrade step -CREATE EXTENSION citus VERSION '6.2-1'; +-- Create extension in oldest version +CREATE EXTENSION citus VERSION '5.0'; +ALTER EXTENSION citus UPDATE TO '5.0-1'; +ALTER EXTENSION citus UPDATE TO '5.0-2'; +ALTER EXTENSION citus UPDATE TO '5.1-1'; +ALTER EXTENSION citus UPDATE TO '5.1-2'; +ALTER EXTENSION citus UPDATE TO '5.1-3'; +ALTER EXTENSION citus UPDATE TO '5.1-4'; +ALTER EXTENSION citus UPDATE TO '5.1-5'; +ALTER EXTENSION citus UPDATE TO '5.1-6'; +ALTER EXTENSION citus UPDATE TO '5.1-7'; +ALTER EXTENSION citus UPDATE TO '5.1-8'; +ALTER EXTENSION citus UPDATE TO '5.2-1'; +ALTER EXTENSION citus UPDATE TO '5.2-2'; +ALTER EXTENSION citus UPDATE TO '5.2-3'; +ALTER EXTENSION citus UPDATE TO '5.2-4'; +ALTER EXTENSION citus UPDATE TO '6.0-1'; +ALTER EXTENSION citus UPDATE TO '6.0-2'; +ALTER EXTENSION citus UPDATE TO '6.0-3'; +ALTER EXTENSION citus UPDATE TO '6.0-4'; +ALTER EXTENSION citus UPDATE TO '6.0-5'; +ALTER EXTENSION citus UPDATE TO '6.0-6'; +ALTER EXTENSION citus UPDATE TO '6.0-7'; +ALTER EXTENSION citus UPDATE TO '6.0-8'; +ALTER EXTENSION citus UPDATE TO '6.0-9'; +ALTER EXTENSION citus UPDATE TO '6.0-10'; +ALTER EXTENSION citus UPDATE TO '6.0-11'; +ALTER EXTENSION citus UPDATE TO '6.0-12'; +ALTER EXTENSION citus UPDATE TO '6.0-13'; +ALTER EXTENSION citus UPDATE TO '6.0-14'; +ALTER EXTENSION citus UPDATE TO '6.0-15'; +ALTER EXTENSION citus UPDATE TO '6.0-16'; +ALTER EXTENSION citus UPDATE TO '6.0-17'; +ALTER EXTENSION citus UPDATE TO '6.0-18'; +ALTER EXTENSION citus UPDATE TO '6.1-1'; +ALTER EXTENSION citus UPDATE TO '6.1-2'; +ALTER EXTENSION citus UPDATE TO '6.1-3'; +ALTER EXTENSION citus UPDATE TO '6.1-4'; +ALTER EXTENSION citus UPDATE TO '6.1-5'; +ALTER EXTENSION citus UPDATE TO '6.1-6'; +ALTER EXTENSION citus UPDATE TO '6.1-7'; +ALTER EXTENSION citus UPDATE TO '6.1-8'; +ALTER EXTENSION citus UPDATE TO '6.1-9'; +ALTER EXTENSION citus UPDATE TO '6.1-10'; +ALTER EXTENSION citus UPDATE TO '6.1-11'; +ALTER EXTENSION citus UPDATE TO '6.1-12'; +ALTER EXTENSION citus UPDATE TO '6.1-13'; +ALTER EXTENSION citus UPDATE TO '6.1-14'; +ALTER EXTENSION citus UPDATE TO '6.1-15'; +ALTER EXTENSION citus UPDATE TO '6.1-16'; +ALTER EXTENSION citus UPDATE TO '6.1-17'; +ALTER EXTENSION citus UPDATE TO '6.2-1'; ALTER EXTENSION citus UPDATE TO '6.2-2'; -- show running version @@ -44,7 +93,11 @@ WHERE pgd.refclassid = 'pg_extension'::regclass AND pge.extname = 'citus' AND pgio.schema NOT IN ('pg_catalog', 'citus'); --- drop extension an re-create in newest version +-- see incompatible version errors out DROP EXTENSION citus; \c +CREATE EXTENSION citus VERSION '5.0'; + +-- re-create in newest version +\c CREATE EXTENSION citus; From ad3fbd9689c5ef153743132c166b529a5699fccf Mon Sep 17 00:00:00 2001 From: Jason Petersen Date: Tue, 4 Apr 2017 15:07:22 -0600 Subject: [PATCH 4/8] Refactor utility-skip/extn-check code This was getting pretty long and complex in the context of the main utility hook. Moved out the checks for what should skip Citus process- ing and what should have version checks performed. --- .../distributed/executor/multi_utility.c | 131 ++++++++++-------- 1 file changed, 76 insertions(+), 55 deletions(-) diff --git a/src/backend/distributed/executor/multi_utility.c b/src/backend/distributed/executor/multi_utility.c index fa1a3cd51..51c02ad11 100644 --- a/src/backend/distributed/executor/multi_utility.c +++ b/src/backend/distributed/executor/multi_utility.c @@ -95,6 +95,10 @@ struct DropRelationCallbackState }; +/* Local functions forward declarations for deciding when to perform processing/checks */ +static bool SkipCitusProcessingForUtility(Node *parsetree); +static bool IsCitusExtensionStmt(Node *parsetree); + /* Local functions forward declarations for Transmit statement */ static bool IsTransmitStmt(Node *parsetree); static void VerifyTransmitStmt(CopyStmt *copyStatement); @@ -171,64 +175,12 @@ multi_ProcessUtility(Node *parsetree, Oid savedUserId = InvalidOid; int savedSecurityContext = 0; List *ddlJobs = NIL; - bool extensionStatement = false; - bool checkExtensionVersion = false; + bool skipCitusProcessing = SkipCitusProcessingForUtility(parsetree); - if (IsA(parsetree, TransactionStmt)) + if (skipCitusProcessing) { - /* - * Transaction statements (e.g. ABORT, COMMIT) can be run in aborted - * transactions in which case a lot of checks cannot be done safely in - * that state. Since we never need to intercept transaction statements, - * skip our checks and immediately fall into standard_ProcessUtility. - */ - standard_ProcessUtility(parsetree, queryString, context, - params, dest, completionTag); + bool checkExtensionVersion = IsCitusExtensionStmt(parsetree); - return; - } - - if (IsA(parsetree, CreateExtensionStmt)) - { - CreateExtensionStmt *createExtensionStmt = (CreateExtensionStmt *) parsetree; - if (strcmp(createExtensionStmt->extname, "citus") == 0) - { - checkExtensionVersion = true; - } - - extensionStatement = true; - } - - if (IsA(parsetree, AlterExtensionStmt)) - { - AlterExtensionStmt *alterExtensionStmt = (AlterExtensionStmt *) parsetree; - if (strcmp(alterExtensionStmt->extname, "citus") == 0) - { - checkExtensionVersion = true; - } - - extensionStatement = true; - } - - if (IsA(parsetree, DropStmt)) - { - DropStmt *dropStatement = (DropStmt *) parsetree; - - if (dropStatement->removeType == OBJECT_EXTENSION) - { - extensionStatement = true; - } - } - - if (extensionStatement || IsA(parsetree, VariableSetStmt)) - { - /* - * In CitusHasBeenLoaded check below, we compare binary Citus version, - * extension version and available version. If they are different, we - * force user to execute ALTER EXTENSION citus UPDATE. Therefore, we - * drop to standard_ProcessUtility earlier for some commands which are - * safe and necessary to user even in version mismatch situation. - */ standard_ProcessUtility(parsetree, queryString, context, params, dest, completionTag); @@ -456,6 +408,75 @@ multi_ProcessUtility(Node *parsetree, } +static bool +SkipCitusProcessingForUtility(Node *parsetree) +{ + switch (parsetree->type) + { + /* + * In the CitusHasBeenLoaded check, we compare versions of loaded code, + * the installed extension, and available extension. If they differ, we + * force user to execute ALTER EXTENSION citus UPDATE. To allow this, + * CREATE/DROP/ALTER extension must be omitted from Citus processing. + */ + case T_DropStmt: + { + DropStmt *dropStatement = (DropStmt *) parsetree; + + if (dropStatement->removeType != OBJECT_EXTENSION) + { + return false; + } + } + + /* no break, fall through */ + + case T_CreateExtensionStmt: + case T_AlterExtensionStmt: + + /* + * Transaction statements (e.g. ABORT, COMMIT) can be run in aborted + * transactions in which case a lot of checks cannot be done safely in + * that state. Since we never need to intercept transaction statements, + * skip our checks and immediately fall into standard_ProcessUtility. + */ + case T_TransactionStmt: + + /* + * Skip processing of variable set statements, to allow changing the + * enable_version_checks GUC during testing. + */ + case T_VariableSetStmt: + { + return true; + } + + default: + { + return false; + } + } +} + + +static bool +IsCitusExtensionStmt(Node *parsetree) +{ + char *extensionName = ""; + + if (IsA(parsetree, CreateExtensionStmt)) + { + extensionName = ((CreateExtensionStmt *) parsetree)->extname; + } + else if (IsA(parsetree, AlterExtensionStmt)) + { + extensionName = ((AlterExtensionStmt *) parsetree)->extname; + } + + return (strcmp(extensionName, "citus") == 0); +} + + /* Is the passed in statement a transmit statement? */ static bool IsTransmitStmt(Node *parsetree) From ef81b21a49000666b29eba26518089da4dd53b31 Mon Sep 17 00:00:00 2001 From: Jason Petersen Date: Tue, 4 Apr 2017 15:36:19 -0600 Subject: [PATCH 5/8] Clean up ErrorIfUnstableCreateOrAlterExtensionStmt Swaps an Assert in for an ereport, and adds details and hints to the error message to help users with a possibly confusing scenario. --- .../distributed/executor/multi_utility.c | 62 ++++++++++++------- src/test/regress/expected/multi_extension.out | 4 +- 2 files changed, 43 insertions(+), 23 deletions(-) diff --git a/src/backend/distributed/executor/multi_utility.c b/src/backend/distributed/executor/multi_utility.c index 51c02ad11..8986d6a11 100644 --- a/src/backend/distributed/executor/multi_utility.c +++ b/src/backend/distributed/executor/multi_utility.c @@ -135,6 +135,7 @@ static bool OptionsSpecifyOwnedBy(List *optionList, Oid *ownedByTableId); static void ErrorIfDistributedRenameStmt(RenameStmt *renameStatement); /* Local functions forward declarations for helper functions */ +static char * ExtractNewExtensionVersion(Node *parsetree); static void CreateLocalTable(RangeVar *relation, char *nodeName, int32 nodePort); static bool IsAlterTableRenameStmt(RenameStmt *renameStatement); static void ExecuteDistributedDDLJob(DDLJob *ddlJob); @@ -1341,23 +1342,53 @@ DeparseVacuumColumnNames(List *columnNameList) static void ErrorIfUnstableCreateOrAlterExtensionStmt(Node *parsetree) { + char *newExtensionVersion = ExtractNewExtensionVersion(parsetree); + + if (newExtensionVersion != NULL) + { + /* explicit version provided in CREATE or ALTER EXTENSION UPDATE; verify */ + if (!MajorVersionsCompatible(newExtensionVersion, CITUS_EXTENSIONVERSION)) + { + ereport(ERROR, (errmsg("specified version incompatible with loaded " + "Citus library"), + errdetail("Loaded library requires %s, but %s was specified.", + CITUS_MAJORVERSION, newExtensionVersion), + errhint("If a newer library is present, restart the database " + "and try the command again."))); + } + } + else + { + /* + * No version was specified, so PostgreSQL will use the default_version + * from the citus.control file. In case a new default is available, we + * will force a compatibility check of the latest available version. + */ + availableExtensionVersion = NULL; + ErrorIfAvailableVersionMismatch(); + } +} + + +static char * +ExtractNewExtensionVersion(Node *parsetree) +{ + char *newVersion = NULL; List *optionsList = NIL; ListCell *optionsCell = NULL; if (IsA(parsetree, CreateExtensionStmt)) { - CreateExtensionStmt *createExtensionStmt = (CreateExtensionStmt *) parsetree; - optionsList = createExtensionStmt->options; + optionsList = ((CreateExtensionStmt *) parsetree)->options; } else if (IsA(parsetree, AlterExtensionStmt)) { - AlterExtensionStmt *alterExtensionStmt = (AlterExtensionStmt *) parsetree; - optionsList = alterExtensionStmt->options; + optionsList = ((AlterExtensionStmt *) parsetree)->options; } else { - ereport(ERROR, (errmsg("unsupported node type, create or alter extension " - "is expected"))); + /* input must be one of the two above types */ + Assert(false); } foreach(optionsCell, optionsList) @@ -1366,25 +1397,12 @@ ErrorIfUnstableCreateOrAlterExtensionStmt(Node *parsetree) if (strcmp(defElement->defname, "new_version") == 0) { - char *newVersion = strVal(defElement->arg); - - if (!MajorVersionsCompatible(newVersion, CITUS_EXTENSIONVERSION)) - { - ereport(ERROR, (errmsg("requested version is not compatible with " - "loaded Citus binaries."))); - } - - return; + newVersion = strVal(defElement->arg); + break; } } - /* - * new_version is not specified in ALTER EXTENSION statement, PostgreSQL will use - * default_version from citus.control file. We will flush availableExtensionVersion - * to re-check the available version from citus.control file. - */ - availableExtensionVersion = NULL; - ErrorIfAvailableVersionMismatch(); + return newVersion; } diff --git a/src/test/regress/expected/multi_extension.out b/src/test/regress/expected/multi_extension.out index 6448fdbda..c5a85b304 100644 --- a/src/test/regress/expected/multi_extension.out +++ b/src/test/regress/expected/multi_extension.out @@ -103,7 +103,9 @@ WHERE pgd.refclassid = 'pg_extension'::regclass AND DROP EXTENSION citus; \c CREATE EXTENSION citus VERSION '5.0'; -ERROR: requested version is not compatible with loaded Citus binaries. +ERROR: specified version incompatible with loaded Citus library +DETAIL: Loaded library requires 6.2, but 5.0 was specified. +HINT: If a newer library is present, restart the database and try the command again. -- re-create in newest version \c CREATE EXTENSION citus; From 033fda91833269a32fe746e37221b71b416e2ff6 Mon Sep 17 00:00:00 2001 From: Jason Petersen Date: Tue, 4 Apr 2017 15:59:10 -0600 Subject: [PATCH 6/8] Clean up remaining error messages Added details and hints, based off of similar PostgreSQL scenarios. --- src/backend/distributed/utils/metadata_cache.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/src/backend/distributed/utils/metadata_cache.c b/src/backend/distributed/utils/metadata_cache.c index 33026bb74..275441320 100644 --- a/src/backend/distributed/utils/metadata_cache.c +++ b/src/backend/distributed/utils/metadata_cache.c @@ -1013,8 +1013,13 @@ ErrorIfAvailableVersionMismatch(void) availableVersion = AvailableExtensionVersion(); if (!MajorVersionsCompatible(availableVersion, CITUS_EXTENSIONVERSION)) { - ereport(ERROR, (errmsg("server restart is needed because, loaded Citus binaries " - "does not match the available extension version"))); + ereport(ERROR, (errmsg("loaded Citus library version differs from latest " + "available extension version"), + errdetail("Loaded library requires %s, but the latest control " + "file specifies %s.", CITUS_MAJORVERSION, + availableVersion), + errhint("Restart the database to load the latest Citus " + "library."))); } } @@ -1038,9 +1043,12 @@ ErrorIfInstalledVersionMismatch(void) installedVersion = InstalledExtensionVersion(); if (!MajorVersionsCompatible(installedVersion, CITUS_EXTENSIONVERSION)) { - ereport(ERROR, (errmsg("\"ALTER EXTENSION citus UPDATE;\" is needed, because " - "loaded Citus binaries does not match the installed " - "extension version"))); + ereport(ERROR, (errmsg("loaded Citus library version differs from installed " + "extension version"), + errdetail("Loaded library requires %s, but the installed " + "extension version is %s.", CITUS_MAJORVERSION, + installedVersion), + errhint("Run ALTER EXTENSION citus UPDATE and try again."))); } } From 7e46f41c120a6890a5136ed3ad7ebfeb64c4aa96 Mon Sep 17 00:00:00 2001 From: Jason Petersen Date: Tue, 4 Apr 2017 16:15:39 -0600 Subject: [PATCH 7/8] Add comments, use strncmp, clean up GUC desc. Good to go! --- .../distributed/executor/multi_utility.c | 19 +++++++++++++++++-- src/backend/distributed/shared_library_init.c | 4 ++-- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/src/backend/distributed/executor/multi_utility.c b/src/backend/distributed/executor/multi_utility.c index 8986d6a11..fad9cad76 100644 --- a/src/backend/distributed/executor/multi_utility.c +++ b/src/backend/distributed/executor/multi_utility.c @@ -409,6 +409,12 @@ multi_ProcessUtility(Node *parsetree, } +/* + * SkipCitusProcessingForUtility simply returns whether a given utility should + * bypass Citus processing and checks and be handled exclusively by standard + * PostgreSQL utility processing. At present, CREATE/ALTER/DROP EXTENSION, + * ABORT, COMMIT, ROLLBACK, and SET (GUC) statements are exempt from Citus. + */ static bool SkipCitusProcessingForUtility(Node *parsetree) { @@ -460,6 +466,11 @@ SkipCitusProcessingForUtility(Node *parsetree) } +/* + * IsCitusExtensionStmt returns whether a given utility is a CREATE or ALTER + * EXTENSION statement which references the citus extension. This function + * returns false for all other inputs. + */ static bool IsCitusExtensionStmt(Node *parsetree) { @@ -1370,6 +1381,11 @@ ErrorIfUnstableCreateOrAlterExtensionStmt(Node *parsetree) } +/* + * ExtractNewExtensionVersion returns the new extension version specified by + * a CREATE or ALTER EXTENSION statement. Other inputs are not permitted. This + * function returns NULL for statements with no explicit version specified. + */ static char * ExtractNewExtensionVersion(Node *parsetree) { @@ -1394,8 +1410,7 @@ ExtractNewExtensionVersion(Node *parsetree) foreach(optionsCell, optionsList) { DefElem *defElement = (DefElem *) lfirst(optionsCell); - - if (strcmp(defElement->defname, "new_version") == 0) + if (strncmp(defElement->defname, "new_version", NAMEDATALEN) == 0) { newVersion = strVal(defElement->arg); break; diff --git a/src/backend/distributed/shared_library_init.c b/src/backend/distributed/shared_library_init.c index fdd428bb2..c3dcf4e88 100644 --- a/src/backend/distributed/shared_library_init.c +++ b/src/backend/distributed/shared_library_init.c @@ -615,7 +615,7 @@ RegisterCitusConfigVariables(void) DefineCustomStringVariable( "citus.version", - gettext_noop("Shows the running Citus version"), + gettext_noop("Shows the Citus library version"), NULL, &CitusVersion, CITUS_VERSION, @@ -625,7 +625,7 @@ RegisterCitusConfigVariables(void) DefineCustomBoolVariable( "citus.enable_version_checks", - gettext_noop("Enables version checks in CREATE/ALTER extension queries"), + gettext_noop("Enables version checks during CREATE/ALTER EXTENSION commands"), NULL, &EnableVersionChecks, true, From 8b4620ef16726dff1231f71216911daae10ae0da Mon Sep 17 00:00:00 2001 From: Jason Petersen Date: Tue, 4 Apr 2017 16:40:17 -0600 Subject: [PATCH 8/8] Use RESET for GUC test, not reconnect More limited in what it does, better test. --- src/test/regress/expected/multi_extension.out | 2 +- src/test/regress/sql/multi_extension.sql | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/regress/expected/multi_extension.out b/src/test/regress/expected/multi_extension.out index c5a85b304..3daa82f42 100644 --- a/src/test/regress/expected/multi_extension.out +++ b/src/test/regress/expected/multi_extension.out @@ -100,8 +100,8 @@ WHERE pgd.refclassid = 'pg_extension'::regclass AND (1 row) -- see incompatible version errors out +RESET citus.enable_version_checks; DROP EXTENSION citus; -\c CREATE EXTENSION citus VERSION '5.0'; ERROR: specified version incompatible with loaded Citus library DETAIL: Loaded library requires 6.2, but 5.0 was specified. diff --git a/src/test/regress/sql/multi_extension.sql b/src/test/regress/sql/multi_extension.sql index 3f1746c54..0be9c5096 100644 --- a/src/test/regress/sql/multi_extension.sql +++ b/src/test/regress/sql/multi_extension.sql @@ -94,8 +94,8 @@ WHERE pgd.refclassid = 'pg_extension'::regclass AND pgio.schema NOT IN ('pg_catalog', 'citus'); -- see incompatible version errors out +RESET citus.enable_version_checks; DROP EXTENSION citus; -\c CREATE EXTENSION citus VERSION '5.0'; -- re-create in newest version