Skip to content

Commit

Permalink
Fix error in show_chunks
Browse files Browse the repository at this point in the history
If "created_after/before" is used with a "time" type partitioning
column then show_chunks was not showing appropriate list due to a
mismatch in the comparison of the "creation_time" metadata (which is
stored as a timestamptz) with the internally converted epoch based
input argument value. This is now fixed by not doing the unnecessary
conversion into the internal format for cases where it's not needed.

Fixes #6611
  • Loading branch information
nikkhils committed Mar 7, 2024
1 parent a8666d0 commit 6b819de
Show file tree
Hide file tree
Showing 6 changed files with 84 additions and 21 deletions.
2 changes: 2 additions & 0 deletions .unreleased/fix_6617
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Fixes: #6617 Fix error in show_chunks
Thanks: @kevcenteno for reporting an issue with the show_chunks API showing incorrect output when 'created_before/created_after' was used with time-partitioned columns.
29 changes: 23 additions & 6 deletions src/chunk.c
Original file line number Diff line number Diff line change
Expand Up @@ -2081,7 +2081,8 @@ ts_chunk_show_chunks(PG_FUNCTION_ARGS)

/*
* We cannot have a mix of [older_than/newer_than] and [created_before/created_after].
* So, check that first.
* So, check that first. Note that created_before/created_after have a type of
* TIMESTAMPTZOID regardless of the partitioning type.
*/

if (!PG_ARGISNULL(3))
Expand All @@ -2094,7 +2095,11 @@ ts_chunk_show_chunks(PG_FUNCTION_ARGS)
"or \"created_after\"")));

arg_type = get_fn_expr_argtype(fcinfo->flinfo, 3);
created_before = ts_time_value_from_arg(PG_GETARG_DATUM(3), arg_type, time_type, false);
/* We use the existing function for various type/conversion checks */
created_before =
ts_time_value_from_arg(PG_GETARG_DATUM(3), arg_type, TIMESTAMPTZOID, false);
/* convert into int64 format for comparisons */
created_before = ts_internal_to_time_int64(created_before, TIMESTAMPTZOID);
before_after = true;
}

Expand All @@ -2108,7 +2113,11 @@ ts_chunk_show_chunks(PG_FUNCTION_ARGS)
"or \"created_after\"")));

arg_type = get_fn_expr_argtype(fcinfo->flinfo, 4);
created_after = ts_time_value_from_arg(PG_GETARG_DATUM(4), arg_type, time_type, false);
/* We use the existing function for various type/conversion checks */
created_after =
ts_time_value_from_arg(PG_GETARG_DATUM(4), arg_type, TIMESTAMPTZOID, false);
/* convert into int64 format for comparisons */
created_after = ts_internal_to_time_int64(created_after, TIMESTAMPTZOID);
before_after = true;
}

Expand Down Expand Up @@ -4152,7 +4161,8 @@ ts_chunk_drop_chunks(PG_FUNCTION_ARGS)

/*
* We cannot have a mix of [older_than/newer_than] and [created_before/created_after].
* So, check that first.
* So, check that first. Note that created_before/created_after have a type of
* TIMESTAMPTZOID regardless of the partitioning type.
*/

if (!PG_ARGISNULL(4))
Expand All @@ -4170,7 +4180,11 @@ ts_chunk_drop_chunks(PG_FUNCTION_ARGS)
" partitioning.")));

arg_type = get_fn_expr_argtype(fcinfo->flinfo, 4);
created_before = ts_time_value_from_arg(PG_GETARG_DATUM(4), arg_type, time_type, false);
/* We use the existing function for various type/conversion checks */
created_before =
ts_time_value_from_arg(PG_GETARG_DATUM(4), arg_type, TIMESTAMPTZOID, false);
/* convert into int64 format for comparisons */
created_before = ts_internal_to_time_int64(created_before, TIMESTAMPTZOID);
before_after = true;
older_than = created_before;
}
Expand All @@ -4189,7 +4203,10 @@ ts_chunk_drop_chunks(PG_FUNCTION_ARGS)
"with \"integer\"-like"
" partitioning.")));
arg_type = get_fn_expr_argtype(fcinfo->flinfo, 5);
created_after = ts_time_value_from_arg(PG_GETARG_DATUM(5), arg_type, time_type, false);
/* We use the existing function for various type/conversion checks */
created_after = ts_time_value_from_arg(PG_GETARG_DATUM(5), arg_type, TIMESTAMPTZOID, false);
/* convert into int64 format for comparisons */
created_after = ts_internal_to_time_int64(created_after, TIMESTAMPTZOID);
before_after = true;
newer_than = created_after;
}
Expand Down
38 changes: 29 additions & 9 deletions test/expected/chunk_utils.out
Original file line number Diff line number Diff line change
Expand Up @@ -862,14 +862,6 @@ SELECT * FROM test.show_subtables('drop_chunk_test_tstz');
_timescaledb_internal._hyper_5_22_chunk |
(2 rows)

-- "created_before/after" can be used with time partitioning in show chunks
SELECT show_chunks('drop_chunk_test_tstz', created_before => now() + INTERVAL '1 hour');
show_chunks
-----------------------------------------
_timescaledb_internal._hyper_5_21_chunk
_timescaledb_internal._hyper_5_22_chunk
(2 rows)

BEGIN;
SELECT show_chunks('drop_chunk_test_ts');
show_chunks
Expand Down Expand Up @@ -1528,11 +1520,39 @@ ORDER BY chunk_name ;
_timescaledb_internal | _hyper_12_41_chunk
(1 row)

-- "created_before/after" can be used with time partitioning in drop chunks
-- "created_before/after" can be used with time partitioning in drop/show chunks
SELECT show_chunks('drop_chunk_test_tstz', created_before => now() - INTERVAL '1 hour');
show_chunks
-------------
(0 rows)

SELECT drop_chunks('drop_chunk_test_tstz', created_before => now() + INTERVAL '1 hour');
drop_chunks
-----------------------------------------
_timescaledb_internal._hyper_5_21_chunk
_timescaledb_internal._hyper_5_22_chunk
(2 rows)

SELECT show_chunks('drop_chunk_test_ts');
show_chunks
-----------------------------------------
_timescaledb_internal._hyper_4_19_chunk
_timescaledb_internal._hyper_4_20_chunk
(2 rows)

-- "created_before/after" accept timestamptz even though partitioning col is just
-- timestamp
SELECT show_chunks('drop_chunk_test_ts', created_after => now() - INTERVAL '1 hour', created_before => now());
show_chunks
-----------------------------------------
_timescaledb_internal._hyper_4_19_chunk
_timescaledb_internal._hyper_4_20_chunk
(2 rows)

SELECT drop_chunks('drop_chunk_test_ts', created_after => INTERVAL '1 hour', created_before => now());
drop_chunks
-----------------------------------------
_timescaledb_internal._hyper_4_19_chunk
_timescaledb_internal._hyper_4_20_chunk
(2 rows)

10 changes: 7 additions & 3 deletions test/sql/chunk_utils.sql
Original file line number Diff line number Diff line change
Expand Up @@ -308,8 +308,6 @@ INSERT INTO PUBLIC.drop_chunk_test_tstz VALUES(now()+INTERVAL '5 minutes', 1.0,

SELECT * FROM test.show_subtables('drop_chunk_test_ts');
SELECT * FROM test.show_subtables('drop_chunk_test_tstz');
-- "created_before/after" can be used with time partitioning in show chunks
SELECT show_chunks('drop_chunk_test_tstz', created_before => now() + INTERVAL '1 hour');

BEGIN;
SELECT show_chunks('drop_chunk_test_ts');
Expand Down Expand Up @@ -649,5 +647,11 @@ FROM timescaledb_information.chunks
WHERE hypertable_name = 'hyper1' and hypertable_schema = 'test1'
ORDER BY chunk_name ;

-- "created_before/after" can be used with time partitioning in drop chunks
-- "created_before/after" can be used with time partitioning in drop/show chunks
SELECT show_chunks('drop_chunk_test_tstz', created_before => now() - INTERVAL '1 hour');
SELECT drop_chunks('drop_chunk_test_tstz', created_before => now() + INTERVAL '1 hour');
SELECT show_chunks('drop_chunk_test_ts');
-- "created_before/after" accept timestamptz even though partitioning col is just
-- timestamp
SELECT show_chunks('drop_chunk_test_ts', created_after => now() - INTERVAL '1 hour', created_before => now());
SELECT drop_chunks('drop_chunk_test_ts', created_after => INTERVAL '1 hour', created_before => now());
17 changes: 16 additions & 1 deletion tsl/test/expected/policy_generalization.out
Original file line number Diff line number Diff line change
Expand Up @@ -144,13 +144,28 @@ SELECT DISTINCT compression_status FROM _timescaledb_internal.compressed_chunk_s
(1 row)

-- Compression policy
SELECT add_compression_policy('test', compress_created_before => INTERVAL '2 seconds') AS compress_chunks_job_id \gset
SELECT add_compression_policy('test', compress_created_before => INTERVAL '1 hour') AS compress_chunks_job_id \gset
SELECT pg_sleep(3);
pg_sleep
----------

(1 row)

CALL run_job(:compress_chunks_job_id);
-- Chunk compression status
SELECT DISTINCT compression_status FROM _timescaledb_internal.compressed_chunk_stats;
compression_status
--------------------
Uncompressed
(1 row)

SELECT remove_compression_policy('test');
remove_compression_policy
---------------------------
t
(1 row)

SELECT add_compression_policy('test', compress_created_before => INTERVAL '2 seconds') AS compress_chunks_job_id \gset
CALL run_job(:compress_chunks_job_id);
-- Chunk compression status
SELECT DISTINCT compression_status FROM _timescaledb_internal.compressed_chunk_stats;
Expand Down
9 changes: 7 additions & 2 deletions tsl/test/sql/policy_generalization.sql
Original file line number Diff line number Diff line change
Expand Up @@ -62,12 +62,17 @@ INSERT INTO test SELECT i, i %10, 0.10 FROM generate_series(1, 100, 1) i;
SELECT DISTINCT compression_status FROM _timescaledb_internal.compressed_chunk_stats;

-- Compression policy
SELECT add_compression_policy('test', compress_created_before => INTERVAL '2 seconds') AS compress_chunks_job_id \gset
SELECT add_compression_policy('test', compress_created_before => INTERVAL '1 hour') AS compress_chunks_job_id \gset
SELECT pg_sleep(3);
CALL run_job(:compress_chunks_job_id);

-- Chunk compression status
SELECT DISTINCT compression_status FROM _timescaledb_internal.compressed_chunk_stats;
SELECT remove_compression_policy('test');
SELECT add_compression_policy('test', compress_created_before => INTERVAL '2 seconds') AS compress_chunks_job_id \gset
CALL run_job(:compress_chunks_job_id);
-- Chunk compression status
SELECT DISTINCT compression_status FROM _timescaledb_internal.compressed_chunk_stats;

-- check for WARNING/NOTICE if policy already exists
SELECT add_compression_policy('test', compress_created_before => INTERVAL '2 seconds',
if_not_exists => true);
Expand Down

0 comments on commit 6b819de

Please sign in to comment.