From 71929b8f8626b18f48e18d543e62726edfddf074 Mon Sep 17 00:00:00 2001 From: Mats Kindahl Date: Tue, 24 Sep 2024 15:50:16 +0200 Subject: [PATCH] Fix ALTER TABLE spurious printout If `create_compress_chunk()` is called as part of a utility statement, a spurious printout will happen because `ts_alter_table_with_event_trigger()` uses a `List` node rather than a proper statement which causes `EventTriggerAlterTableEnd()` to not recognize the node and print out an error message. This is fixed by calling `AlterTableInternal` instead of `ts_alter_table_with_event_trigger()` for utility statements and make sure that `EventTriggerAlterTableStart()` and `EventTriggerAlterTableEnd()` are called for non-utility statements calling `create_compress_chunk()` with a dummy statement. --- src/chunk.c | 2 +- tsl/src/compression/api.c | 27 +++++++++++++++++++++++ tsl/src/compression/compression_storage.c | 5 +++-- 3 files changed, 31 insertions(+), 3 deletions(-) diff --git a/src/chunk.c b/src/chunk.c index 00a8dc0ef65..07c797eaf29 100644 --- a/src/chunk.c +++ b/src/chunk.c @@ -663,7 +663,7 @@ set_attoptions(Relation ht_rel, Oid chunk_oid) if (alter_cmds != NIL) { - ts_alter_table_with_event_trigger(chunk_oid, NULL, alter_cmds, false); + AlterTableInternal(chunk_oid, alter_cmds, false); list_free_deep(alter_cmds); } } diff --git a/tsl/src/compression/api.c b/tsl/src/compression/api.c index e07bb7d5b2c..d9fe0a45b00 100644 --- a/tsl/src/compression/api.c +++ b/tsl/src/compression/api.c @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -60,6 +61,15 @@ typedef struct CompressChunkCxt static Oid get_compressed_chunk_index_for_recompression(Chunk *uncompressed_chunk); static Oid recompress_chunk_segmentwise_impl(Chunk *chunk); +static Node * +create_dummy_query() +{ + RawStmt *query = NULL; + query = makeNode(RawStmt); + query->stmt = (Node *) makeNode(SelectStmt); + return (Node *) query; +} + static void compression_chunk_size_catalog_insert(int32 src_chunk_id, const RelationSize *src_size, int32 compress_chunk_id, const RelationSize *compress_size, @@ -414,6 +424,14 @@ compress_chunk_impl(Oid hypertable_relid, Oid chunk_relid) mergable_chunk = find_chunk_to_merge_into(cxt.srcht, cxt.srcht_chunk); if (!mergable_chunk) { + /* + * Set up a dummy parsetree since we're calling AlterTableInternal + * inside create_compress_chunk(). We can use anything here because we + * are not calling EventTriggerDDLCommandEnd but we use a parse tree + * type that CreateCommandTag can handle to avoid spurious printouts + * in the event that EventTriggerDDLCommandEnd is called. + */ + EventTriggerAlterTableStart(create_dummy_query()); /* create compressed chunk and a new table */ compress_ht_chunk = create_compress_chunk(cxt.compress_ht, cxt.srcht_chunk, InvalidOid); new_compressed_chunk = true; @@ -421,6 +439,7 @@ compress_chunk_impl(Oid hypertable_relid, Oid chunk_relid) (errmsg("new compressed chunk \"%s.%s\" created", NameStr(compress_ht_chunk->fd.schema_name), NameStr(compress_ht_chunk->fd.table_name)))); + EventTriggerAlterTableEnd(); } else { @@ -677,8 +696,16 @@ tsl_create_compressed_chunk(PG_FUNCTION_ARGS) /* Acquire locks on catalog tables to keep till end of txn */ LockRelationOid(catalog_get_table_id(ts_catalog_get(), CHUNK), RowExclusiveLock); + /* + * Set up a dummy parsetree since we're calling AlterTableInternal inside + * create_compress_chunk(). We can use anything here because we are not + * calling EventTriggerDDLCommandEnd but we use a parse tree type that + * CreateCommandTag can handle to avoid spurious printouts. + */ + EventTriggerAlterTableStart(create_dummy_query()); /* Create compressed chunk using existing table */ compress_ht_chunk = create_compress_chunk(cxt.compress_ht, cxt.srcht_chunk, chunk_table); + EventTriggerAlterTableEnd(); /* Copy chunk constraints (including fkey) to compressed chunk */ ts_chunk_constraints_create(cxt.compress_ht, compress_ht_chunk); diff --git a/tsl/src/compression/compression_storage.c b/tsl/src/compression/compression_storage.c index eb1d7741efc..3d28ac561f5 100644 --- a/tsl/src/compression/compression_storage.c +++ b/tsl/src/compression/compression_storage.c @@ -165,7 +165,8 @@ set_toast_tuple_target_on_chunk(Oid compressed_table_id) .subtype = AT_SetRelOptions, .def = (Node *) list_make1(&def_elem), }; - ts_alter_table_with_event_trigger(compressed_table_id, NULL, list_make1(&cmd), true); + + AlterTableInternal(compressed_table_id, list_make1(&cmd), true); } static void @@ -270,7 +271,7 @@ modify_compressed_toast_table_storage(CompressionSettings *settings, List *colde if (cmds != NIL) { - ts_alter_table_with_event_trigger(compress_relid, NULL, cmds, false); + AlterTableInternal(compress_relid, cmds, false); } }