diff --git a/.unreleased/pr_7789 b/.unreleased/pr_7789 new file mode 100644 index 00000000000..b71fdafde10 --- /dev/null +++ b/.unreleased/pr_7789 @@ -0,0 +1,2 @@ +Implements: #7789 Do not recompress segmentwise when default order by is empty +Fixes: #7748 Crash in the segmentwise recompression diff --git a/tsl/src/compression/api.c b/tsl/src/compression/api.c index 7f61b6865af..6b8d89206ba 100644 --- a/tsl/src/compression/api.c +++ b/tsl/src/compression/api.c @@ -940,12 +940,14 @@ tsl_compress_chunk_wrapper(Chunk *chunk, bool if_not_compressed, bool recompress if (ts_chunk_is_compressed(chunk)) { + CompressionSettings *chunk_settings = ts_compression_settings_get(chunk->table_id); + bool valid_orderby_settings = chunk_settings->fd.orderby; if (recompress) { CompressionSettings *ht_settings = ts_compression_settings_get(chunk->hypertable_relid); - CompressionSettings *chunk_settings = ts_compression_settings_get(chunk->table_id); - if (!ts_compression_settings_equal(ht_settings, chunk_settings)) + if (!valid_orderby_settings || + !ts_compression_settings_equal(ht_settings, chunk_settings)) { decompress_chunk_impl(chunk, false); compress_chunk_impl(chunk->hypertable_relid, chunk->table_id); @@ -962,17 +964,21 @@ tsl_compress_chunk_wrapper(Chunk *chunk, bool if_not_compressed, bool recompress return uncompressed_chunk_id; } - if (ts_guc_enable_segmentwise_recompression && ts_chunk_is_partial(chunk) && - get_compressed_chunk_index_for_recompression(chunk)) + if (ts_guc_enable_segmentwise_recompression && valid_orderby_settings && + ts_chunk_is_partial(chunk) && get_compressed_chunk_index_for_recompression(chunk)) { uncompressed_chunk_id = recompress_chunk_segmentwise_impl(chunk); } else { - if (!ts_guc_enable_segmentwise_recompression) + if (!ts_guc_enable_segmentwise_recompression || !valid_orderby_settings) elog(NOTICE, - "segmentwise recompression is disabled, performing full recompression on " + "segmentwise recompression is disabled%s, performing full " + "recompression on " "chunk \"%s.%s\"", + (ts_guc_enable_segmentwise_recompression && !valid_orderby_settings ? + " due to no order by" : + ""), NameStr(chunk->fd.schema_name), NameStr(chunk->fd.table_name)); decompress_chunk_impl(chunk, false); diff --git a/tsl/src/compression/create.c b/tsl/src/compression/create.c index be722ec035f..f9fc867bd51 100644 --- a/tsl/src/compression/create.c +++ b/tsl/src/compression/create.c @@ -1142,10 +1142,16 @@ compression_setting_orderby_get_default(Hypertable *ht, ArrayType *segmentby) else orderby = ""; - elog(NOTICE, - "default order by for hypertable \"%s\" is set to \"%s\"", - get_rel_name(ht->main_table_relid), - orderby); + if (*orderby == '\0') + ereport(NOTICE, + (errmsg("default order by for hypertable \"%s\" is set to \"\"", + get_rel_name(ht->main_table_relid))), + errdetail("Segmentwise recompression will be disabled")); + else + elog(NOTICE, + "default order by for hypertable \"%s\" is set to \"%s\"", + get_rel_name(ht->main_table_relid), + orderby); elog(LOG_SERVER_ONLY, "order_by default: hypertable=\"%s\" clauses=\"%s\" function=\"%s.%s\" confidence=%d", diff --git a/tsl/src/compression/recompress.c b/tsl/src/compression/recompress.c index a268e74d09a..609d57a59d1 100644 --- a/tsl/src/compression/recompress.c +++ b/tsl/src/compression/recompress.c @@ -18,6 +18,7 @@ #include "compression.h" #include "compression_dml.h" #include "create.h" +#include "debug_assert.h" #include "guc.h" #include "hypercore/hypercore_handler.h" #include "hypercore/utils.h" @@ -83,6 +84,15 @@ tsl_recompress_chunk_segmentwise(PG_FUNCTION_ARGS) "enable it by first setting " "timescaledb.enable_segmentwise_recompression to on"))); } + CompressionSettings *settings = ts_compression_settings_get(uncompressed_chunk_id); + if (!settings->fd.orderby) + { + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("segmentwise recompression cannot be applied for " + "compression with no " + "order by"))); + } uncompressed_chunk_id = recompress_chunk_segmentwise_impl(chunk); } @@ -112,6 +122,10 @@ recompress_chunk_segmentwise_impl(Chunk *uncompressed_chunk) Chunk *compressed_chunk = ts_chunk_get_by_id(uncompressed_chunk->fd.compressed_chunk_id, true); CompressionSettings *settings = ts_compression_settings_get(uncompressed_chunk->table_id); + /* We should not do segment-wise recompression with empty orderby, see #7748 + */ + Ensure(settings->fd.orderby, "empty order by, cannot recompress segmentwise"); + /* new status after recompress should simply be compressed (1) * It is ok to update this early on in the transaction as it keeps a lock * on the updated tuple in the CHUNK table potentially preventing other transaction diff --git a/tsl/test/expected/recompress_chunk_segmentwise.out b/tsl/test/expected/recompress_chunk_segmentwise.out index 9f42ea8993b..1e327ca2f3b 100644 --- a/tsl/test/expected/recompress_chunk_segmentwise.out +++ b/tsl/test/expected/recompress_chunk_segmentwise.out @@ -647,13 +647,51 @@ INFO: Using index "compress_hyper_19_20_chunk__ts_meta_min_1__ts_meta_max_1_idx _timescaledb_internal._hyper_18_19_chunk (1 row) +-- Test behaviour when default order by is empty: should not segmentwise-recompress in this case +CREATE TABLE no_oby(ts int, c1 int); +SELECT create_hypertable('no_oby','ts'); +NOTICE: adding not-null constraint to column "ts" + create_hypertable +---------------------- + (20,public,no_oby,t) +(1 row) + +\set VERBOSITY default +ALTER TABLE no_oby SET (timescaledb.compress, timescaledb.compress_segmentby = 'ts'); +NOTICE: default order by for hypertable "no_oby" is set to "" +DETAIL: Segmentwise recompression will be disabled +\set VERBOSITY terse +INSERT INTO no_oby (ts,c1) VALUES (6,6); +SELECT show_chunks as chunk_to_compress FROM show_chunks('no_oby') LIMIT 1 \gset +SELECT compress_chunk(:'chunk_to_compress'); +INFO: using tuplesort to scan rows from "_hyper_20_21_chunk" for compression + compress_chunk +------------------------------------------ + _timescaledb_internal._hyper_20_21_chunk +(1 row) + +INSERT INTO no_oby (ts,c1) VALUES (7,7); +-- Direct segmentwise-recompress request: throw an error +\set ON_ERROR_STOP 0 +SELECT _timescaledb_functions.recompress_chunk_segmentwise(:'chunk_to_compress'); +ERROR: segmentwise recompression cannot be applied for compression with no order by +\set ON_ERROR_STOP 1 +-- Compress wrapper: do full recompress instead of by segment +SELECT compress_chunk(:'chunk_to_compress'); +NOTICE: segmentwise recompression is disabled due to no order by, performing full recompression on chunk "_timescaledb_internal._hyper_20_21_chunk" +INFO: using tuplesort to scan rows from "_hyper_20_21_chunk" for compression + compress_chunk +------------------------------------------ + _timescaledb_internal._hyper_20_21_chunk +(1 row) + RESET timescaledb.debug_compression_path_info; --- Test behaviour when enable_segmentwise_recompression GUC if OFF CREATE TABLE guc_test(time timestamptz not null, a int, b int, c int); SELECT create_hypertable('guc_test', by_range('time', INTERVAL '1 day')); create_hypertable ------------------- - (20,t) + (22,t) (1 row) ALTER TABLE guc_test set (timescaledb.compress, timescaledb.compress_segmentby = 'a, b'); @@ -663,7 +701,7 @@ SELECT show_chunks as chunk_to_compress FROM show_chunks('guc_test') LIMIT 1 \gs SELECT compress_chunk(:'chunk_to_compress'); compress_chunk ------------------------------------------ - _timescaledb_internal._hyper_20_21_chunk + _timescaledb_internal._hyper_22_24_chunk (1 row) INSERT INTO guc_test VALUES ('2024-10-30 14:14:00.501519-06'::timestamptz, 1, 1, 2); @@ -675,9 +713,9 @@ ERROR: segmentwise recompression functionality disabled, enable it by first set \set ON_ERROR_STOP 1 -- When GUC is OFF, entire chunk should be fully uncompressed and compressed instead SELECT compress_chunk(:'chunk_to_compress'); -NOTICE: segmentwise recompression is disabled, performing full recompression on chunk "_timescaledb_internal._hyper_20_21_chunk" +NOTICE: segmentwise recompression is disabled, performing full recompression on chunk "_timescaledb_internal._hyper_22_24_chunk" compress_chunk ------------------------------------------ - _timescaledb_internal._hyper_20_21_chunk + _timescaledb_internal._hyper_22_24_chunk (1 row) diff --git a/tsl/test/sql/recompress_chunk_segmentwise.sql b/tsl/test/sql/recompress_chunk_segmentwise.sql index 9cd179a50c8..22521831cb2 100644 --- a/tsl/test/sql/recompress_chunk_segmentwise.sql +++ b/tsl/test/sql/recompress_chunk_segmentwise.sql @@ -307,6 +307,25 @@ INSERT INTO noseg VALUES ('2025-01-24 10:54:27.323421-07'::timestamptz, 1, 1, 2) -- should recompress chunk using the default index SELECT compress_chunk(:'chunk_to_compress'); +-- Test behaviour when default order by is empty: should not segmentwise-recompress in this case +CREATE TABLE no_oby(ts int, c1 int); +SELECT create_hypertable('no_oby','ts'); +\set VERBOSITY default +ALTER TABLE no_oby SET (timescaledb.compress, timescaledb.compress_segmentby = 'ts'); +\set VERBOSITY terse + +INSERT INTO no_oby (ts,c1) VALUES (6,6); +SELECT show_chunks as chunk_to_compress FROM show_chunks('no_oby') LIMIT 1 \gset +SELECT compress_chunk(:'chunk_to_compress'); + +INSERT INTO no_oby (ts,c1) VALUES (7,7); +-- Direct segmentwise-recompress request: throw an error +\set ON_ERROR_STOP 0 +SELECT _timescaledb_functions.recompress_chunk_segmentwise(:'chunk_to_compress'); +\set ON_ERROR_STOP 1 +-- Compress wrapper: do full recompress instead of by segment +SELECT compress_chunk(:'chunk_to_compress'); + RESET timescaledb.debug_compression_path_info; --- Test behaviour when enable_segmentwise_recompression GUC if OFF