Skip to content

Commit

Permalink
Do not recompress segmentwise when default order by is empty (#7789)
Browse files Browse the repository at this point in the history
When we have compression where default order by is set to "", we cannot
do meaningful segmentwise recompression. Therefore we will return error
when segmentwise recompression is requested directly and will do full
recompression when general compression is requested.

Fixes: #7748
  • Loading branch information
natalya-aksman authored Mar 4, 2025
1 parent 9115e12 commit d7c4244
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 d7c4244

Please sign in to comment.