Fix crashes in functions using AlterTableInternal

A number of TimescaleDB functions internally call `AlterTableInternal`
to modify tables or indexes. For instance, `compress_chunk` and
`attach_tablespace` act as DDL commands to modify
hypertables. However, crashes occur when these functions are called
via `SELECT * INTO FROM <function_name>` or the equivalent `CREATE
TABLE AS` statement. The crashes happen because these statements are
considered process utility commands and therefore sets up an event
trigger context for collecting commands. However, the event trigger
context is not properly set up to record alter table statements in
this code path, thus causing the crashes.

To prevent crashes, wrap `AlterTableInternal` with the event trigger
functions to properly initialize the event trigger context.

src/chunk.c

@@ -624,6 +624,7 @@ set_attoptions(Relation ht_rel, Oid chunk_oid)
 	TupleDesc tupleDesc = RelationGetDescr(ht_rel);
 	int natts = tupleDesc->natts;
 	int attno;
+	List *alter_cmds = NIL;
 
 	for (attno = 1; attno <= natts; attno++)
 	{
@@ -654,7 +655,7 @@ set_attoptions(Relation ht_rel, Oid chunk_oid)
 			cmd->subtype = AT_SetOptions;
 			cmd->name = attributeName;
 			cmd->def = (Node *) untransformRelOptions(options);
-			AlterTableInternal(chunk_oid, list_make1(cmd), false);
+			alter_cmds = lappend(alter_cmds, cmd);
 		}
 
 		/*
@@ -674,12 +675,18 @@ set_attoptions(Relation ht_rel, Oid chunk_oid)
 				cmd->subtype = AT_SetStatistics;
 				cmd->name = attributeName;
 				cmd->def = (Node *) makeInteger(target);
-				AlterTableInternal(chunk_oid, list_make1(cmd), false);
+				alter_cmds = lappend(alter_cmds, cmd);
 			}
 		}
 
 		ReleaseSysCache(tuple);
 	}
+
+	if (alter_cmds != NIL)
+	{
+		ts_alter_table_with_event_trigger(chunk_oid, NULL, alter_cmds, false);
+		list_free_deep(alter_cmds);
+	}
 }
 
 static void
@@ -1141,7 +1148,7 @@ chunk_table_drop_inherit(const Chunk *chunk, Hypertable *ht)
 		.missing_ok = false
 	};
 
-	AlterTableInternal(chunk->table_id, list_make1(&drop_inh_cmd), false);
+	ts_alter_table_with_event_trigger(chunk->table_id, NULL, list_make1(&drop_inh_cmd), false);
 }
 
 /*

src/chunk_index.c

@@ -1060,7 +1060,7 @@ chunk_index_tuple_set_tablespace(TupleInfo *ti, void *data)
 	cmd->name = tablespace;
 	cmds = lappend(cmds, cmd);
 
-	AlterTableInternal(indexrelid, cmds, false);
+	ts_alter_table_with_event_trigger(indexrelid, NULL, cmds, false);
 
 	if (should_free)
 		heap_freetuple(tuple);
@@ -1294,7 +1294,7 @@ ts_chunk_index_move_all(Oid chunk_relid, Oid index_tblspc)
 	foreach (lc, indexlist)
 	{
 		Oid chunk_idxoid = lfirst_oid(lc);
-		AlterTableInternal(chunk_idxoid, list_make1(&cmd), false);
+		ts_alter_table_with_event_trigger(chunk_idxoid, NULL, list_make1(&cmd), false);
 	}
 	table_close(chunkrel, AccessShareLock);
 }

src/dimension.c

@@ -1078,9 +1078,7 @@ dimension_add_not_null_on_column(Oid table_relid, char *colname)
 			(errmsg("adding not-null constraint to column \"%s\"", colname),
 			 errdetail("Time dimensions cannot have NULL values.")));
 
-	EventTriggerAlterTableStart((Node *) &cmd);
-	AlterTableInternal(table_relid, list_make1(&cmd), false);
-	EventTriggerAlterTableEnd();
+	ts_alter_table_with_event_trigger(table_relid, (Node *) &cmd, list_make1(&cmd), false);
 }
 
 void

src/ts_catalog/tablespace.c

@@ -22,6 +22,7 @@
 #include "scanner.h"
 #include "ts_catalog/tablespace.h"
 #include "compat/compat.h"
+#include "utils.h"
 
 #define TABLESPACE_DEFAULT_CAPACITY 4
 
@@ -502,8 +503,7 @@ ts_tablespace_attach(PG_FUNCTION_ARGS)
 
 		cmd->subtype = AT_SetTableSpace;
 		cmd->name = NameStr(*tspcname);
-
-		AlterTableInternal(hypertable_oid, list_make1(cmd), false);
+		ts_alter_table_with_event_trigger(hypertable_oid, fcinfo->context, list_make1(cmd), false);
 	}
 	relation_close(rel, AccessShareLock);
 	PG_RETURN_VOID();
@@ -644,7 +644,7 @@ tablespace_detach_all(Oid hypertable_oid)
 }
 
 static void
-detach_tablespace_from_hypertable_if_set(Oid hypertable_oid, Oid tspcoid)
+detach_tablespace_from_hypertable_if_set(Node *detach_cmd, Oid hypertable_oid, Oid tspcoid)
 {
 	Relation rel;
 
@@ -657,8 +657,7 @@ detach_tablespace_from_hypertable_if_set(Oid hypertable_oid, Oid tspcoid)
 
 		cmd->subtype = AT_SetTableSpace;
 		cmd->name = "pg_default";
-
-		AlterTableInternal(hypertable_oid, list_make1(cmd), false);
+		ts_alter_table_with_event_trigger(hypertable_oid, detach_cmd, list_make1(cmd), false);
 	}
 	relation_close(rel, AccessShareLock);
 }
@@ -696,7 +695,7 @@ ts_tablespace_detach(PG_FUNCTION_ARGS)
 	if (OidIsValid(hypertable_oid))
 	{
 		ret = tablespace_detach_one(hypertable_oid, NameStr(*tspcname), tspcoid, if_attached);
-		detach_tablespace_from_hypertable_if_set(hypertable_oid, tspcoid);
+		detach_tablespace_from_hypertable_if_set(fcinfo->context, hypertable_oid, tspcoid);
 	}
 	else
 	{
@@ -707,7 +706,8 @@ ts_tablespace_detach(PG_FUNCTION_ARGS)
 		foreach (cell, hypertables)
 		{
 			const int32 hypertable_id = lfirst_int(cell);
-			detach_tablespace_from_hypertable_if_set(ts_hypertable_id_to_relid(hypertable_id),
+			detach_tablespace_from_hypertable_if_set(fcinfo->context,
+													 ts_hypertable_id_to_relid(hypertable_id),
 													 tspcoid);
 		}
 	}
@@ -736,7 +736,7 @@ ts_tablespace_detach_all_from_hypertable(PG_FUNCTION_ARGS)
 		elog(ERROR, "invalid argument");
 
 	result = tablespace_detach_all(hypertable_relid);
-	AlterTableInternal(hypertable_relid, list_make1(cmd), false);
+	ts_alter_table_with_event_trigger(hypertable_relid, fcinfo->context, list_make1(cmd), false);
 
 	PG_RETURN_INT32(result);
 }

src/utils.c

@@ -16,6 +16,8 @@
 #include <catalog/pg_inherits.h>
 #include <catalog/pg_operator.h>
 #include <catalog/pg_type.h>
+#include <commands/event_trigger.h>
+#include <commands/tablecmds.h>
 #include <fmgr.h>
 #include <funcapi.h>
 #include <nodes/makefuncs.h>
@@ -1113,3 +1115,27 @@ ts_get_relnatts(Oid relid)
 	ReleaseSysCache(tp);
 	return result;
 }
+
+/*
+ * Wrap AlterTableInternal() for event trigger handling.
+ *
+ * AlterTableInternal can be called as a utility command, which is common in a
+ * SQL function that alters a table in some form when called in the form
+ * SELECT <cmd> INTO <table>. This is transformed into a process utility
+ * command (CREATE TABLE AS), which expects an event trigger context to be
+ * set up.
+ *
+ * The "cmd" parameter can be set to a higher-level command that caused the
+ * alter table to occur. If "cmd" is set to NULL, the "cmds" list will be used
+ * instead.
+ */
+void
+ts_alter_table_with_event_trigger(Oid relid, Node *cmd, List *cmds, bool recurse)
+{
+	if (cmd == NULL)
+		cmd = (Node *) cmds;
+
+	EventTriggerAlterTableStart(cmd);
+	AlterTableInternal(relid, cmds, recurse);
+	EventTriggerAlterTableEnd();
+}

src/utils.h

@@ -196,5 +196,7 @@ extern TSDLLEXPORT RelationSize ts_relation_size_impl(Oid relid);
 
 extern TSDLLEXPORT const char *ts_get_node_name(Node *node);
 extern TSDLLEXPORT int ts_get_relnatts(Oid relid);
+extern TSDLLEXPORT void ts_alter_table_with_event_trigger(Oid relid, Node *cmd, List *cmds,
+														  bool recurse);
 
 #endif /* TIMESCALEDB_UTILS_H */

test/expected/tablespace.out

@@ -199,13 +199,9 @@ ERROR:  permission denied for tablespace "tablespace1" by table owner "default_p
 SET ROLE :ROLE_DEFAULT_PERM_USER;
 GRANT CREATE ON TABLESPACE tablespace1 TO :ROLE_DEFAULT_PERM_USER_2;
 SET ROLE :ROLE_DEFAULT_PERM_USER_2;
---should work fine now
-SELECT attach_tablespace('tablespace1', 'tspace_1dim');
- attach_tablespace 
--------------------
- 
-(1 row)
-
+--should work fine now. Test SELECT INTO utility statements to ensure
+--internal alter table function call works with event triggers.
+SELECT true INTO attached FROM attach_tablespace('tablespace1', 'tspace_1dim');
 SELECT attach_tablespace('tablespace2', 'tspace_1dim');
  attach_tablespace 
 -------------------
@@ -283,8 +279,9 @@ SELECT * FROM hypertable_tablespaces;
  tspace_2dim | tablespace1
 (2 rows)
 
-SELECT detach_tablespace('tablespace1');
+SELECT * INTO detached FROM detach_tablespace('tablespace1');
 NOTICE:  tablespace "tablespace1" remains attached to 1 hypertable(s) due to lack of permissions
+SELECT * FROM detached;
  detach_tablespace 
 -------------------
                  1
@@ -369,7 +366,8 @@ ERROR:  cannot revoke privilege while tablespace "tablespace2" is attached to hy
 SET ROLE :ROLE_DEFAULT_PERM_USER_2;
 --set other user should make detach work
 SET ROLE :ROLE_DEFAULT_PERM_USER;
-SELECT detach_tablespaces('tspace_2dim');
+SELECT * INTO detached_all FROM detach_tablespaces('tspace_2dim');
+SELECT * FROM detached_all;
  detach_tablespaces 
 --------------------
                   2

test/sql/tablespace.sql

@@ -100,8 +100,9 @@ SELECT attach_tablespace('tablespace1', 'tspace_1dim');
 SET ROLE :ROLE_DEFAULT_PERM_USER;
 GRANT CREATE ON TABLESPACE tablespace1 TO :ROLE_DEFAULT_PERM_USER_2;
 SET ROLE :ROLE_DEFAULT_PERM_USER_2;
---should work fine now
-SELECT attach_tablespace('tablespace1', 'tspace_1dim');
+--should work fine now. Test SELECT INTO utility statements to ensure
+--internal alter table function call works with event triggers.
+SELECT true INTO attached FROM attach_tablespace('tablespace1', 'tspace_1dim');
 SELECT attach_tablespace('tablespace2', 'tspace_1dim');
 -- Tablespace for tspace_1dim should be set and attached
 SELECT * FROM hypertable_tablespaces WHERE hypertable = 'tspace_1dim';
@@ -133,7 +134,8 @@ SELECT detach_tablespace('tablespace1', 'tspace_2dim');
 --detach tablespace1 from all tables. Should only detach from
 --'tspace_1dim' (1 tablespace) due to lack of permissions
 SELECT * FROM hypertable_tablespaces;
-SELECT detach_tablespace('tablespace1');
+SELECT * INTO detached FROM detach_tablespace('tablespace1');
+SELECT * FROM detached;
 SELECT * FROM _timescaledb_catalog.tablespace;
 SELECT * FROM show_tablespaces('tspace_1dim');
 SELECT * FROM show_tablespaces('tspace_2dim');
@@ -164,7 +166,8 @@ SET ROLE :ROLE_DEFAULT_PERM_USER_2;
 
 --set other user should make detach work
 SET ROLE :ROLE_DEFAULT_PERM_USER;
-SELECT detach_tablespaces('tspace_2dim');
+SELECT * INTO detached_all FROM detach_tablespaces('tspace_2dim');
+SELECT * FROM detached_all;
 SELECT * FROM _timescaledb_catalog.tablespace;
 SELECT * FROM show_tablespaces('tspace_1dim');
 SELECT * FROM show_tablespaces('tspace_2dim');

tsl/src/compression/compress_utils.c

@@ -180,7 +180,7 @@ preserve_uncompressed_chunk_stats(Oid chunk_relid)
 	};
 
 	ExecVacuum(NULL, &vs, true);
-	AlterTableInternal(chunk_relid, list_make1(&at_cmd), false);
+	ts_alter_table_with_event_trigger(chunk_relid, NULL, list_make1(&at_cmd), false);
 }
 
 /* This function is intended to undo the disabling of autovacuum done when we compressed a chunk.
@@ -205,7 +205,10 @@ restore_autovacuum_on_decompress(Oid uncompressed_hypertable_relid, Oid uncompre
 				makeDefElem("autovacuum_enabled", (Node *) makeString("true"), -1)),
 		};
 
-		AlterTableInternal(uncompressed_chunk_relid, list_make1(&at_cmd), false);
+		ts_alter_table_with_event_trigger(uncompressed_chunk_relid,
+										  NULL,
+										  list_make1(&at_cmd),
+										  false);
 	}
 }
 

tsl/src/compression/create.c

@@ -370,7 +370,7 @@ modify_compressed_toast_table_storage(CompressColInfo *cc, Oid compress_relid)
 	}
 	if (cmds != NIL)
 	{
-		AlterTableInternal(compress_relid, cmds, false);
+		ts_alter_table_with_event_trigger(compress_relid, NULL, cmds, false);
 	}
 }
 
@@ -527,7 +527,7 @@ set_toast_tuple_target_on_compressed(Oid compressed_table_id)
 		.subtype = AT_SetRelOptions,
 		.def = (Node *) list_make1(&def_elem),
 	};
-	AlterTableInternal(compressed_table_id, list_make1(&cmd), true);
+	ts_alter_table_with_event_trigger(compressed_table_id, NULL, list_make1(&cmd), true);
 }
 
 static int32
@@ -963,7 +963,7 @@ add_column_to_compression_table(Hypertable *compress_ht, CompressColInfo *compre
 	addcol_cmd->missing_ok = false;
 
 	/* alter the table and add column */
-	AlterTableInternal(compress_relid, list_make1(addcol_cmd), true);
+	ts_alter_table_with_event_trigger(compress_relid, NULL, list_make1(addcol_cmd), true);
 	modify_compressed_toast_table_storage(compress_cols, compress_relid);
 }
 
@@ -982,7 +982,7 @@ drop_column_from_compression_table(Hypertable *compress_ht, char *name)
 	cmd->missing_ok = true;
 
 	/* alter the table and drop column */
-	AlterTableInternal(compress_relid, list_make1(cmd), true);
+	ts_alter_table_with_event_trigger(compress_relid, NULL, list_make1(cmd), true);
 }
 
 /*

tsl/src/reorder.c

@@ -13,6 +13,7 @@
 
 /* see postgres commit ab5e9caa4a3ec4765348a0482e88edcf3f6aab4a */
 
+#include "utils.h"
 #include <postgres.h>
 #include <access/amapi.h>
 #include <access/multixact.h>
@@ -185,8 +186,11 @@ tsl_move_chunk(PG_FUNCTION_ARGS)
 					 errmsg("ignoring index parameter"),
 					 errdetail("Chunk will not be reordered as it has compressed data.")));
 
-		AlterTableInternal(chunk_id, list_make1(&cmd), false);
-		AlterTableInternal(compressed_chunk->table_id, list_make1(&cmd), false);
+		ts_alter_table_with_event_trigger(chunk_id, fcinfo->context, list_make1(&cmd), false);
+		ts_alter_table_with_event_trigger(compressed_chunk->table_id,
+										  fcinfo->context,
+										  list_make1(&cmd),
+										  false);
 		/* move indexes on original and compressed chunk */
 		ts_chunk_index_move_all(chunk_id, index_destination_tablespace);
 		ts_chunk_index_move_all(compressed_chunk->table_id, index_destination_tablespace);

tsl/test/expected/compression_ddl.out

@@ -1161,6 +1161,10 @@ WHERE reltablespace in
  test2
 (3 rows)
 
+-- test compress_chunk() with utility statement (SELECT ... INTO)
+SELECT compress_chunk(ch) INTO compressed_chunks FROM show_chunks('test2') ch;
+SELECT decompress_chunk(ch) INTO decompressed_chunks FROM show_chunks('test2') ch;
+-- compress again
 SELECT compress_chunk(ch) FROM show_chunks('test2') ch;
               compress_chunk               
 -------------------------------------------
@@ -1180,7 +1184,7 @@ WHERE reltablespace in
 (1 row)
 
 DROP TABLE test2 CASCADE;
-NOTICE:  drop cascades to table _timescaledb_internal.compress_hyper_17_104_chunk
+NOTICE:  drop cascades to table _timescaledb_internal.compress_hyper_17_105_chunk
 DROP TABLESPACE tablespace2;
 -- Create a table with a compressed table and then delete the
 -- compressed table and see that the drop of the hypertable does not

tsl/test/expected/move.out

@@ -251,14 +251,9 @@ ERROR:  must be owner of hypertable "cluster_test"
 GRANT CREATE ON TABLESPACE tablespace2 TO :ROLE_DEFAULT_PERM_USER;
 \c :TEST_DBNAME :ROLE_DEFAULT_PERM_USER
 -- move with chunk index for reorder
-SELECT move_chunk(chunk=>'_timescaledb_internal._hyper_1_2_chunk', destination_tablespace=>'tablespace1', index_destination_tablespace=>'tablespace1', reorder_index=>'_timescaledb_internal._hyper_1_2_chunk_cluster_test_time_idx', verbose=>TRUE);
+SELECT true INTO move_chunk_result FROM move_chunk(chunk=>'_timescaledb_internal._hyper_1_2_chunk', destination_tablespace=>'tablespace1', index_destination_tablespace=>'tablespace1', reorder_index=>'_timescaledb_internal._hyper_1_2_chunk_cluster_test_time_idx', verbose=>TRUE);
 INFO:  reordering "_timescaledb_internal._hyper_1_2_chunk" using sequential scan and sort
 INFO:  "_hyper_1_2_chunk": found 0 removable, 5 nonremovable row versions in 1 pages
- move_chunk 
-------------
- 
-(1 row)
-
 SELECT * FROM test.show_subtables('cluster_test');
                  Child                  | Tablespace  
 ----------------------------------------+-------------
@@ -467,12 +462,7 @@ SELECT compress_chunk('_timescaledb_internal._hyper_1_2_chunk') as ch;
  _timescaledb_internal._hyper_1_2_chunk
 (1 row)
 
-SELECT move_chunk(chunk=>'_timescaledb_internal._hyper_1_2_chunk', destination_tablespace=>'tablespace2', index_destination_tablespace=>'tablespace1', verbose=>TRUE);
- move_chunk 
-------------
- 
-(1 row)
-
+SELECT true INTO move_chunk_compressed FROM move_chunk(chunk=>'_timescaledb_internal._hyper_1_2_chunk', destination_tablespace=>'tablespace2', index_destination_tablespace=>'tablespace1', verbose=>TRUE);
 SELECT * FROM test.show_subtables('cluster_test');
                  Child                  | Tablespace  
 ----------------------------------------+-------------

tsl/test/sql/compression_ddl.sql

@@ -478,6 +478,11 @@ SELECT relname FROM pg_class
 WHERE reltablespace in 
   ( SELECT oid from pg_tablespace WHERE spcname = 'tablespace2') ORDER BY 1;
 
+-- test compress_chunk() with utility statement (SELECT ... INTO)
+SELECT compress_chunk(ch) INTO compressed_chunks FROM show_chunks('test2') ch;
+SELECT decompress_chunk(ch) INTO decompressed_chunks FROM show_chunks('test2') ch;
+
+-- compress again
 SELECT compress_chunk(ch) FROM show_chunks('test2') ch;
 
 -- the chunk, compressed chunk + index + toast tables are in tablespace2 now .

tsl/test/sql/move.sql

@@ -64,7 +64,7 @@ GRANT CREATE ON TABLESPACE tablespace2 TO :ROLE_DEFAULT_PERM_USER;
 \c :TEST_DBNAME :ROLE_DEFAULT_PERM_USER
 
 -- move with chunk index for reorder
-SELECT move_chunk(chunk=>'_timescaledb_internal._hyper_1_2_chunk', destination_tablespace=>'tablespace1', index_destination_tablespace=>'tablespace1', reorder_index=>'_timescaledb_internal._hyper_1_2_chunk_cluster_test_time_idx', verbose=>TRUE);
+SELECT true INTO move_chunk_result FROM move_chunk(chunk=>'_timescaledb_internal._hyper_1_2_chunk', destination_tablespace=>'tablespace1', index_destination_tablespace=>'tablespace1', reorder_index=>'_timescaledb_internal._hyper_1_2_chunk_cluster_test_time_idx', verbose=>TRUE);
 SELECT * FROM test.show_subtables('cluster_test');
 SELECT * FROM test.show_indexesp('_timescaledb_internal._hyper%_chunk');
 BEGIN;
@@ -102,7 +102,7 @@ SELECT * FROM test.show_indexesp('_timescaledb_internal._hyper%_chunk');
 --compress chunk and then  move chunk and index to different tablespaces
 ALTER TABLE cluster_test SET (timescaledb.compress, timescaledb.compress_segmentby = 'location');
 SELECT compress_chunk('_timescaledb_internal._hyper_1_2_chunk') as ch;
-SELECT move_chunk(chunk=>'_timescaledb_internal._hyper_1_2_chunk', destination_tablespace=>'tablespace2', index_destination_tablespace=>'tablespace1', verbose=>TRUE);
+SELECT true INTO move_chunk_compressed FROM move_chunk(chunk=>'_timescaledb_internal._hyper_1_2_chunk', destination_tablespace=>'tablespace2', index_destination_tablespace=>'tablespace1', verbose=>TRUE);
 SELECT * FROM test.show_subtables('cluster_test');
 SELECT * FROM test.show_indexesp('_timescaledb_internal._hyper%_chunk');
 SELECT * FROM test.show_indexesp('_timescaledb_internal.compress_hyper%_chunk');