refactoring, do not emit update-related ERROR on SET & ALTER EXTENSION pg_pathman (issue #95)

Author: funbringer

src/hooks.c

@@ -613,16 +613,35 @@ pathman_post_parse_analysis_hook(ParseState *pstate, Query *query)
 	if (!pathman_hooks_enabled)
 		return;
 
-	 /* We shouldn't do anything on BEGIN or SET ISOLATION LEVEL stmts */
-	if (query->commandType == CMD_UTILITY &&
-			(xact_is_transaction_stmt(query->utilityStmt) ||
-			 xact_is_set_transaction_stmt(query->utilityStmt)))
-		return;
-
 	/* Finish delayed invalidation jobs */
 	if (IsPathmanReady())
 		finish_delayed_invalidation();
 
+	/*
+	 * We shouldn't proceed on:
+	 *		BEGIN
+	 *		SET [TRANSACTION]
+	 */
+	if (query->commandType == CMD_UTILITY &&
+		   (xact_is_transaction_stmt(query->utilityStmt) ||
+			xact_is_set_stmt(query->utilityStmt)))
+		return;
+
+	/*
+	 * We should also disable pg_pathman on:
+	 *		ALTER EXTENSION pg_pathman
+	 */
+	if (query->commandType == CMD_UTILITY &&
+			xact_is_alter_pathman_stmt(query->utilityStmt))
+	{
+		/* Disable pg_pathman to perform a painless update */
+		(void) set_config_option(PATHMAN_ENABLE, "off",
+								 PGC_SUSET, PGC_S_SESSION,
+								 GUC_ACTION_SAVE, true, 0, false);
+
+		return;
+	}
+
 	/* Load config if pg_pathman exists & it's still necessary */
 	if (IsPathmanEnabled() &&
 		!IsPathmanInitialized() &&

src/include/init.h

@@ -143,14 +143,14 @@ simpify_mcxt_name(MemoryContext mcxt)
 
 
 /* Default column values for PATHMAN_CONFIG_PARAMS */
-#define DEFAULT_ENABLE_PARENT		false
-#define DEFAULT_AUTO				true
-#define DEFAULT_INIT_CALLBACK		InvalidOid
-#define DEFAULT_SPAWN_USING_BGW		false
+#define DEFAULT_PATHMAN_ENABLE_PARENT		false
+#define DEFAULT_PATHMAN_AUTO				true
+#define DEFAULT_PATHMAN_INIT_CALLBACK		InvalidOid
+#define DEFAULT_PATHMAN_SPAWN_USING_BGW		false
 
 /* Other default values (for GUCs etc) */
-#define DEFAULT_PATHMAN_ENABLE		true
-#define DEFAULT_OVERRIDE_COPY		true
+#define DEFAULT_PATHMAN_ENABLE				true
+#define DEFAULT_PATHMAN_OVERRIDE_COPY		true
 
 
 /* Lowest version of Pl/PgSQL frontend compatible with internals (0xAA_BB_CC) */

src/include/pathman.h

@@ -40,6 +40,14 @@
 #endif
 
 
+/*
+ * Main GUC variables.
+ */
+#define PATHMAN_ENABLE					"pg_pathman.enable"
+#define PATHMAN_ENABLE_AUTO_PARTITION	"pg_pathman.enable_auto_partition"
+#define PATHMAN_OVERRIDE_COPY			"pg_pathman.override_copy"
+
+
 /*
  * Definitions for the "pathman_config" table.
  */

src/include/xact_handling.h

@@ -28,7 +28,8 @@ LockAcquireResult xact_lock_rel(Oid relid, LOCKMODE lockmode, bool nowait);
 bool xact_bgw_conflicting_lock_exists(Oid relid);
 bool xact_is_level_read_committed(void);
 bool xact_is_transaction_stmt(Node *stmt);
-bool xact_is_set_transaction_stmt(Node *stmt);
+bool xact_is_set_stmt(Node *stmt);
+bool xact_is_alter_pathman_stmt(Node *stmt);
 bool xact_object_is_visible(TransactionId obj_xmin);
 
 void prevent_data_modification_internal(Oid relid);

src/init.c

@@ -150,8 +150,8 @@ void
 init_main_pathman_toggles(void)
 {
 	/* Main toggle, load_config() will enable it */
-	DefineCustomBoolVariable("pg_pathman.enable",
-							 "Enables pg_pathman's optimizations during the planner stage",
+	DefineCustomBoolVariable(PATHMAN_ENABLE,
+							 "Enables pg_pathman's optimizations during planning stage",
 							 NULL,
 							 &pathman_init_state.pg_pathman_enable,
 							 DEFAULT_PATHMAN_ENABLE,
@@ -162,23 +162,23 @@ init_main_pathman_toggles(void)
 							 NULL);
 
 	/* Global toggle for automatic partition creation */
-	DefineCustomBoolVariable("pg_pathman.enable_auto_partition",
+	DefineCustomBoolVariable(PATHMAN_ENABLE_AUTO_PARTITION,
 							 "Enables automatic partition creation",
 							 NULL,
 							 &pathman_init_state.auto_partition,
-							 DEFAULT_AUTO,
+							 DEFAULT_PATHMAN_AUTO,
 							 PGC_SUSET,
 							 0,
 							 NULL,
 							 NULL,
 							 NULL);
 
 	/* Global toggle for COPY stmt handling */
-	DefineCustomBoolVariable("pg_pathman.override_copy",
+	DefineCustomBoolVariable(PATHMAN_OVERRIDE_COPY,
 							 "Override COPY statement handling",
 							 NULL,
 							 &pathman_init_state.override_copy,
-							 DEFAULT_OVERRIDE_COPY,
+							 DEFAULT_PATHMAN_OVERRIDE_COPY,
 							 PGC_SUSET,
 							 0,
 							 NULL,

src/partition_creation.c

@@ -134,7 +134,7 @@ create_single_range_partition_internal(Oid parent_relid,
 
 	/* Cook args for init_callback */
 	MakeInitCallbackRangeParams(&callback_params,
-								DEFAULT_INIT_CALLBACK,
+								DEFAULT_PATHMAN_INIT_CALLBACK,
 								parent_relid, partition_relid,
 								*start_value, *end_value, value_type);
 
@@ -193,7 +193,7 @@ create_single_hash_partition_internal(Oid parent_relid,
 
 	/* Cook args for init_callback */
 	MakeInitCallbackHashParams(&callback_params,
-							   DEFAULT_INIT_CALLBACK,
+							   DEFAULT_PATHMAN_INIT_CALLBACK,
 							   parent_relid, partition_relid);
 
 	/* Add constraint & execute init_callback */
@@ -263,8 +263,8 @@ create_partitions_for_value(Oid relid, Datum value, Oid value_type)
 	if (pathman_config_contains_relation(relid, NULL, NULL, &rel_xmin, NULL))
 	{
 		/* Take default values */
-		bool	spawn_using_bgw	= DEFAULT_SPAWN_USING_BGW,
-				enable_auto		= DEFAULT_AUTO;
+		bool	spawn_using_bgw	= DEFAULT_PATHMAN_SPAWN_USING_BGW,
+				enable_auto		= DEFAULT_PATHMAN_AUTO;
 
 		/* Values to be extracted from PATHMAN_CONFIG_PARAMS */
 		Datum	values[Natts_pathman_config_params];
@@ -835,7 +835,7 @@ create_table_using_stmt(CreateStmt *create_stmt, Oid relowner)
 	guc_level = NewGUCNestLevel();
 
 	/* ... and set client_min_messages = warning */
-	(void) set_config_option("client_min_messages", "WARNING",
+	(void) set_config_option(CppAsString(client_min_messages), "WARNING",
 							 PGC_USERSET, PGC_S_SESSION,
 							 GUC_ACTION_SAVE, true, 0, false);
 
@@ -1683,7 +1683,7 @@ validate_part_callback(Oid procid, bool emit_error)
 	Form_pg_proc	functup;
 	bool			is_ok = true;
 
-	if (procid == DEFAULT_INIT_CALLBACK)
+	if (procid == DEFAULT_PATHMAN_INIT_CALLBACK)
 		return true;
 
 	tp = SearchSysCache1(PROCOID, ObjectIdGetDatum(procid));

src/pg_pathman.c

@@ -232,8 +232,8 @@ _PG_init(void)
 
 	/* Assign pg_pathman's initial state */
 	temp_init_state.pg_pathman_enable		= DEFAULT_PATHMAN_ENABLE;
-	temp_init_state.auto_partition			= DEFAULT_AUTO;
-	temp_init_state.override_copy			= DEFAULT_OVERRIDE_COPY;
+	temp_init_state.auto_partition			= DEFAULT_PATHMAN_AUTO;
+	temp_init_state.override_copy			= DEFAULT_PATHMAN_OVERRIDE_COPY;
 	temp_init_state.initialization_needed	= true; /* ofc it's needed! */
 
 	/* Apply initial state */

src/relation_info.c

@@ -302,7 +302,7 @@ refresh_pathman_relation_info(Oid relid,
 	/* Else set default values if they cannot be found */
 	else
 	{
-		prel->enable_parent = DEFAULT_ENABLE_PARENT;
+		prel->enable_parent = DEFAULT_PATHMAN_ENABLE_PARENT;
 	}
 
 	/* We've successfully built a cache entry */

src/xact_handling.c

@@ -111,22 +111,36 @@ xact_is_transaction_stmt(Node *stmt)
 }
 
 /*
- * Check if 'stmt' is SET TRANSACTION statement.
+ * Check if 'stmt' is SET [TRANSACTION] statement.
  */
 bool
-xact_is_set_transaction_stmt(Node *stmt)
+xact_is_set_stmt(Node *stmt)
 {
+	/* Check that SET TRANSACTION is implemented via VariableSetStmt */
+	Assert(VAR_SET_MULTI > 0);
+
 	if (!stmt)
 		return false;
 
 	if (IsA(stmt, VariableSetStmt))
-	{
-		VariableSetStmt *var_set_stmt = (VariableSetStmt *) stmt;
+		return true;
 
-		/* special case for SET TRANSACTION ... */
-		if (var_set_stmt->kind == VAR_SET_MULTI)
-			return true;
-	}
+	return false;
+}
+
+/*
+ * Check if 'stmt' is ALTER EXTENSION pg_pathman.
+ */
+bool
+xact_is_alter_pathman_stmt(Node *stmt)
+{
+	if (!stmt)
+		return false;
+
+	if (IsA(stmt, AlterExtensionStmt) &&
+		0 == strcmp(((AlterExtensionStmt *) stmt)->extname,
+					"pg_pathman"))
+		return true;
 
 	return false;
 }