Skip to content

Commit

Permalink
Do not recompress segmentwise when default order by is empty
Browse files Browse the repository at this point in the history
  • Loading branch information
natalya-aksman committed Mar 4, 2025
1 parent 9115e12 commit 1eba81e
Show file tree
Hide file tree
Showing 6 changed files with 99 additions and 14 deletions.
2 changes: 2 additions & 0 deletions .unreleased/pr_7789
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Implements: #7789 Do not recompress segmentwise when default order by is empty
Fixes: #7748 Crash in the segmentwise recompression
18 changes: 12 additions & 6 deletions tsl/src/compression/api.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand Down
14 changes: 10 additions & 4 deletions tsl/src/compression/create.c
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
14 changes: 14 additions & 0 deletions tsl/src/compression/recompress.c
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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
Expand Down
46 changes: 42 additions & 4 deletions tsl/test/expected/recompress_chunk_segmentwise.out
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand All @@ -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);
Expand All @@ -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)

19 changes: 19 additions & 0 deletions tsl/test/sql/recompress_chunk_segmentwise.sql
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 1eba81e

Please sign in to comment.