Skip to content

Commit

Permalink
Fix ALTER TABLE spurious printout
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
mkindahl committed Sep 24, 2024
1 parent d29ccee commit 71929b8
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 3 deletions.
2 changes: 1 addition & 1 deletion src/chunk.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Expand Down
27 changes: 27 additions & 0 deletions tsl/src/compression/api.c
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include <access/tableam.h>
#include <access/xact.h>
#include <catalog/dependency.h>
#include <commands/event_trigger.h>
#include <commands/tablecmds.h>
#include <commands/trigger.h>
#include <libpq-fe.h>
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -414,13 +424,22 @@ 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;
ereport(LOG,
(errmsg("new compressed chunk \"%s.%s\" created",
NameStr(compress_ht_chunk->fd.schema_name),
NameStr(compress_ht_chunk->fd.table_name))));
EventTriggerAlterTableEnd();
}
else
{
Expand Down Expand Up @@ -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);
Expand Down
5 changes: 3 additions & 2 deletions tsl/src/compression/compression_storage.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
}
}

Expand Down

0 comments on commit 71929b8

Please sign in to comment.