Add more tests for compression #5420
Conversation
|
@mkindahl, @nikkhils: please review this pull request.
|
| @@ -32,7 +34,7 @@ SELECT | |||
| \set DECOMPRESS_REVERSE_CMD _timescaledb_internal.decompress_reverse(c::_timescaledb_internal.compressed_data, NULL::BIGINT) | |||
|
|
|||
| -- random order | |||
| CREATE TABLE base_ints AS SELECT row_number() OVER() as rn, item::bigint FROM (select sub.item from (SELECT generate_series(1, 1000) item) as sub ORDER BY gen_rand_minstd()) sub; | |||
| CREATE TABLE base_ints AS SELECT row_number() OVER() as rn, item::bigint FROM (select sub.item from (SELECT generate_series(1, 1000) item) as sub ORDER BY mix(item)) sub; | |||
There was a problem hiding this comment.
gen_rand_minstd() had a global state and updated a table on each call. Changing it to a hash function removes the dependency between test cases, and speeds up the test by about 3 seconds (20%).
| CREATE OR REPLACE FUNCTION _timescaledb_internal.gorilla_compressor_append(internal, DOUBLE PRECISION) | ||
| CREATE OR REPLACE FUNCTION _timescaledb_internal.gorilla_compressor_append(internal, ANYELEMENT) |
There was a problem hiding this comment.
Since the function assume that it is a double, won't this break if you allow other elements to be passed?
from tsl_gorilla_compressor_append:
if (PG_ARGISNULL(1))
gorilla_compressor_append_null(compressor);
else
{
double next_val = PG_GETARG_FLOAT8(1);
gorilla_compressor_append_value(compressor, double_get_bits(next_val));
}There was a problem hiding this comment.
I added the missing files I forgot to commit, it should accept all supported types now. Sorry for the confusion.
tsl/test/src/test_compression.c
Outdated
| gorilla_decompression_iterator_from_datum_forward(PointerGetDatum(compressed), FLOAT8OID); | ||
| for (DecompressResult r = gorilla_decompression_iterator_try_next_forward(iter); !r.is_done; | ||
| r = gorilla_decompression_iterator_try_next_forward(iter)) | ||
| ArrowArray *bulk_result = |
There was a problem hiding this comment.
Are you missing an include here?
There was a problem hiding this comment.
Oh, I missed half the files I wanted to include in this PR, will fix now.
Codecov Report
@@ Coverage Diff @@
## main #5420 +/- ##
==========================================
- Coverage 90.68% 90.68% -0.01%
==========================================
Files 226 227 +1
Lines 52538 52698 +160
==========================================
+ Hits 47645 47790 +145
- Misses 4893 4908 +15
... and 26 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
tsl/test/src/test_compression.c
Outdated
|
|
||
| int64 values[1015]; | ||
| bool nulls[1015]; | ||
| for (int i = 0; i < 1015; i++) |
There was a problem hiding this comment.
I usually use something like this to avoid making mistakes when I change the sizes of the arrays:
| for (int i = 0; i < 1015; i++) | |
| for (int i = 0; i < sizeof(values)/sizeof(*values); i++) |
There was a problem hiding this comment.
Let's just use a define for all of them.
Unit tests for different data sequences, and SQL test for float4.
Unit tests for different data sequences, and SQL test for float4.