Skip to content

Commit 73d67a4

Browse files
committed
Fixing composite bloom filter column naming
As reported in issue #9578, there was a case when concatenating two columns could create overlapping results. For example ('a_b','c') and ('a','b_c') resulted the same composite bloom metadata column name. This caused an error during the compression of the chunk. This change modifies the column naming scheme for composite bloom columns and thus eliminates the issue. Fixes: #9578
1 parent 90d6a34 commit 73d67a4

16 files changed

Lines changed: 281 additions & 128 deletions

.unreleased/pr_9743

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Fixes: #9743 Fixes the composite bloom metadata column naming scheme
2+
Thanks: @pavanmanishd for the first version of the fix

tsl/src/compression/compression_dml.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -269,7 +269,6 @@ init_upsert_bloom_state(ChunkInsertState *cis)
269269

270270
/* Verify bloom column exists in the compressed chunk */
271271
AttrNumber compressed_attnum = get_attnum(compressed_relid, col_name);
272-
Assert(AttributeNumberIsValid(compressed_attnum));
273272
if (!AttributeNumberIsValid(compressed_attnum))
274273
{
275274
continue;

tsl/src/compression/create.c

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -174,8 +174,10 @@ compressed_column_metadata_name_v2(const char *metadata_type, const char **colum
174174
Assert(num_columns <= MAX_BLOOM_FILTER_COLUMNS);
175175

176176
int len = 0;
177-
StringInfoData buf = { 0 };
177+
/* Use a separate buffer for the hash computation */
178+
StringInfoData buf = { 0 }, hash_buf = { 0 };
178179
initStringInfo(&buf);
180+
initStringInfo(&hash_buf);
179181

180182
for (int i = 0; i < num_columns; i++)
181183
{
@@ -187,8 +189,12 @@ compressed_column_metadata_name_v2(const char *metadata_type, const char **colum
187189
if (i > 0)
188190
{
189191
appendStringInfoChar(&buf, '_');
192+
/* The separator for hash purposes needs to be something
193+
* that is not valid in Postgres, hence the zero byte */
194+
appendStringInfoChar(&hash_buf, '\0');
190195
}
191196
appendStringInfo(&buf, "%s", column_names[i]);
197+
appendBinaryStringInfo(&hash_buf, column_names[i], strlen(column_names[i]));
192198
}
193199

194200
len = buf.len;
@@ -197,14 +203,16 @@ compressed_column_metadata_name_v2(const char *metadata_type, const char **colum
197203
* We have to fit the name into NAMEDATALEN - 1 which is 63 bytes:
198204
* 12 (_ts_meta_v2_) + 6 (metadata_type) + [1 (_) + x (column_name)]x num_columns + 1 (_) + 4
199205
* (hash) = 63; x = 63 - 24 = 39.
206+
*
207+
* Fix for bug #9578: we need to differentiate between ('a_b', 'c') and ('a', 'b_c') composite
208+
* column names, for this reason we always go through the hash path for composite column names.
200209
*/
201-
202210
char *result;
203-
if (len > 39)
211+
if (len > 39 || num_columns > 1)
204212
{
205213
const char *errstr = NULL;
206214
char hash[33];
207-
Ensure(pg_md5_hash(buf.data, len, hash, &errstr), "md5 computation failure");
215+
Ensure(pg_md5_hash(hash_buf.data, hash_buf.len, hash, &errstr), "md5 computation failure");
208216
result = psprintf("_ts_meta_v2_%.6s_%.4s_%.39s", metadata_type, hash, buf.data);
209217
}
210218
else

tsl/test/expected/compress_bloom_dml.out

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ select * from dml_test where foo = 1001 and boo = 1002;
3131
Custom Scan (ColumnarScan) on _hyper_1_1_chunk (actual rows=0.00 loops=1)
3232
Vectorized Filter: ((foo = 1001) AND (boo = 1002))
3333
-> Seq Scan on compress_hyper_2_2_chunk (actual rows=0.00 loops=1)
34-
Filter: (_timescaledb_functions.bloom1_contains_any_hashes(regress-test-bloom_boo_foo, TEST-HASHES::bigint[]) AND _timescaledb_functions.bloom1_contains_any_hashes(regress-test-bloom_boo, TEST-HASHES::bigint[]))
34+
Filter: (_timescaledb_functions.bloom1_contains_any_hashes(regress-test-bloom_701f_boo_foo, TEST-HASHES::bigint[]) AND _timescaledb_functions.bloom1_contains_any_hashes(regress-test-bloom_boo, TEST-HASHES::bigint[]))
3535
Rows Removed by Filter: 50
3636

3737
SET timescaledb.enable_dml_bloom_filter = off;

tsl/test/expected/compress_compbloom_basics.out

Lines changed: 65 additions & 65 deletions
Large diffs are not rendered by default.

tsl/test/expected/compress_compbloom_config.out

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -74,15 +74,15 @@ select relid,compress_relid,segmentby,orderby,orderby_desc,orderby_nullsfirst,in
7474

7575
-- Check the auto generated compressed columns
7676
select relname,attname from compressedcols order by 1,2;
77-
relname | attname
78-
--------------------------+--------------------------
77+
relname | attname
78+
--------------------------+-------------------------------
7979
compress_hyper_4_2_chunk | _ts_meta_count
8080
compress_hyper_4_2_chunk | _ts_meta_max_1
8181
compress_hyper_4_2_chunk | _ts_meta_max_2
8282
compress_hyper_4_2_chunk | _ts_meta_min_1
8383
compress_hyper_4_2_chunk | _ts_meta_min_2
84-
compress_hyper_4_2_chunk | regress-test-bloom_a_b_c
8584
compress_hyper_4_2_chunk | regress-test-bloom_c
85+
compress_hyper_4_2_chunk | regress-test-bloom_ed4b_a_b_c
8686
compress_hyper_4_2_chunk | a
8787
compress_hyper_4_2_chunk | b
8888
compress_hyper_4_2_chunk | c
@@ -148,9 +148,9 @@ select relname,attname from compressedcols order by 1,2;
148148
compress_hyper_6_4_chunk | _ts_meta_max_2
149149
compress_hyper_6_4_chunk | _ts_meta_min_1
150150
compress_hyper_6_4_chunk | _ts_meta_min_2
151+
compress_hyper_6_4_chunk | regress-test-bloom_76d9_a_01234567890123456789_b_01234567890123
151152
compress_hyper_6_4_chunk | regress-test-bloom_c_01234567890123456789
152153
compress_hyper_6_4_chunk | regress-test-bloom_d_01234567890123456789
153-
compress_hyper_6_4_chunk | regress-test-bloom_ddd4_a_01234567890123456789_b_01234567890123
154154
compress_hyper_6_4_chunk | regress-test-bloom_e_01234567890123456789
155155
compress_hyper_6_4_chunk | a_01234567890123456789
156156
compress_hyper_6_4_chunk | b_01234567890123456789

tsl/test/expected/compress_compbloom_hash_pushdown.out

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ select * from hash_pushdown_test where a = 1 and b = 2;
3737
Custom Scan (ColumnarScan) on _hyper_1_1_chunk (actual rows=0.00 loops=1)
3838
Vectorized Filter: ((a = 1) AND (b = 2))
3939
-> Seq Scan on compress_hyper_2_2_chunk (actual rows=0.00 loops=1)
40-
Filter: (_timescaledb_functions.bloom1_contains_any_hashes(regress-test-bloom_a_b, TEST-HASHES::bigint[]) AND _timescaledb_functions.bloom1_contains_any_hashes(regress-test-bloom_a, TEST-HASHES::bigint[]))
40+
Filter: (_timescaledb_functions.bloom1_contains_any_hashes(regress-test-bloom_7035_a_b, TEST-HASHES::bigint[]) AND _timescaledb_functions.bloom1_contains_any_hashes(regress-test-bloom_a, TEST-HASHES::bigint[]))
4141
Rows Removed by Filter: 10
4242

4343
-- saop with single bloom

tsl/test/expected/compress_compbloom_index_add.out

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ ORDER BY 1,2,3,4;
9797
-> Custom Scan (ColumnarScan) on _hyper_1_3_chunk (actual rows=0.00 loops=1)
9898
Vectorized Filter: ((a = 1) AND (b = 2))
9999
-> Seq Scan on compress_hyper_2_6_chunk (actual rows=0.00 loops=1)
100-
Filter: (_timescaledb_functions.bloom1_contains_any_hashes(regress-test-bloom_a_b, TEST-HASHES::bigint[]) AND _timescaledb_functions.bloom1_contains_any_hashes(regress-test-bloom_a, TEST-HASHES::bigint[]) AND _timescaledb_functions.bloom1_contains_any_hashes(regress-test-bloom_b, TEST-HASHES::bigint[]))
100+
Filter: (_timescaledb_functions.bloom1_contains_any_hashes(regress-test-bloom_7035_a_b, TEST-HASHES::bigint[]) AND _timescaledb_functions.bloom1_contains_any_hashes(regress-test-bloom_a, TEST-HASHES::bigint[]) AND _timescaledb_functions.bloom1_contains_any_hashes(regress-test-bloom_b, TEST-HASHES::bigint[]))
101101
Rows Removed by Filter: 1
102102

103103
DROP TABLE IF EXISTS mixed_avail_add CASCADE;

tsl/test/expected/compress_compbloom_index_drop.out

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,15 +42,15 @@ ORDER BY 1,2,3,4;
4242
-> Custom Scan (ColumnarScan) on _hyper_1_1_chunk (actual rows=0.00 loops=1)
4343
Vectorized Filter: ((a = 1) AND (b = 2))
4444
-> Seq Scan on compress_hyper_2_4_chunk (actual rows=0.00 loops=1)
45-
Filter: (_timescaledb_functions.bloom1_contains_any_hashes(regress-test-bloom_a_b, TEST-HASHES::bigint[]) AND _timescaledb_functions.bloom1_contains_any_hashes(regress-test-bloom_a, TEST-HASHES::bigint[]) AND _timescaledb_functions.bloom1_contains_any_hashes(regress-test-bloom_b, TEST-HASHES::bigint[]))
45+
Filter: (_timescaledb_functions.bloom1_contains_any_hashes(regress-test-bloom_7035_a_b, TEST-HASHES::bigint[]) AND _timescaledb_functions.bloom1_contains_any_hashes(regress-test-bloom_a, TEST-HASHES::bigint[]) AND _timescaledb_functions.bloom1_contains_any_hashes(regress-test-bloom_b, TEST-HASHES::bigint[]))
4646
Rows Removed by Filter: 2
4747
-> Sort (actual rows=0.00 loops=1)
4848
Sort Key: _hyper_1_2_chunk.ts, _hyper_1_2_chunk.seg
4949
Sort Method: quicksort
5050
-> Custom Scan (ColumnarScan) on _hyper_1_2_chunk (actual rows=0.00 loops=1)
5151
Vectorized Filter: ((a = 1) AND (b = 2))
5252
-> Seq Scan on compress_hyper_2_5_chunk (actual rows=0.00 loops=1)
53-
Filter: (_timescaledb_functions.bloom1_contains_any_hashes(regress-test-bloom_a_b, TEST-HASHES::bigint[]) AND _timescaledb_functions.bloom1_contains_any_hashes(regress-test-bloom_a, TEST-HASHES::bigint[]) AND _timescaledb_functions.bloom1_contains_any_hashes(regress-test-bloom_b, TEST-HASHES::bigint[]))
53+
Filter: (_timescaledb_functions.bloom1_contains_any_hashes(regress-test-bloom_7035_a_b, TEST-HASHES::bigint[]) AND _timescaledb_functions.bloom1_contains_any_hashes(regress-test-bloom_a, TEST-HASHES::bigint[]) AND _timescaledb_functions.bloom1_contains_any_hashes(regress-test-bloom_b, TEST-HASHES::bigint[]))
5454
Rows Removed by Filter: 3
5555
-> Sort (actual rows=0.00 loops=1)
5656
Sort Key: _hyper_1_3_chunk.ts, _hyper_1_3_chunk.seg
@@ -87,15 +87,15 @@ ORDER BY 1,2,3,4;
8787
-> Custom Scan (ColumnarScan) on _hyper_1_1_chunk (actual rows=0.00 loops=1)
8888
Vectorized Filter: ((a = 1) AND (b = 2))
8989
-> Seq Scan on compress_hyper_2_4_chunk (actual rows=0.00 loops=1)
90-
Filter: (_timescaledb_functions.bloom1_contains_any_hashes(regress-test-bloom_a_b, TEST-HASHES::bigint[]) AND _timescaledb_functions.bloom1_contains_any_hashes(regress-test-bloom_a, TEST-HASHES::bigint[]) AND _timescaledb_functions.bloom1_contains_any_hashes(regress-test-bloom_b, TEST-HASHES::bigint[]))
90+
Filter: (_timescaledb_functions.bloom1_contains_any_hashes(regress-test-bloom_7035_a_b, TEST-HASHES::bigint[]) AND _timescaledb_functions.bloom1_contains_any_hashes(regress-test-bloom_a, TEST-HASHES::bigint[]) AND _timescaledb_functions.bloom1_contains_any_hashes(regress-test-bloom_b, TEST-HASHES::bigint[]))
9191
Rows Removed by Filter: 2
9292
-> Sort (actual rows=0.00 loops=1)
9393
Sort Key: _hyper_1_2_chunk.ts, _hyper_1_2_chunk.seg
9494
Sort Method: quicksort
9595
-> Custom Scan (ColumnarScan) on _hyper_1_2_chunk (actual rows=0.00 loops=1)
9696
Vectorized Filter: ((a = 1) AND (b = 2))
9797
-> Seq Scan on compress_hyper_2_5_chunk (actual rows=0.00 loops=1)
98-
Filter: (_timescaledb_functions.bloom1_contains_any_hashes(regress-test-bloom_a_b, TEST-HASHES::bigint[]) AND _timescaledb_functions.bloom1_contains_any_hashes(regress-test-bloom_a, TEST-HASHES::bigint[]) AND _timescaledb_functions.bloom1_contains_any_hashes(regress-test-bloom_b, TEST-HASHES::bigint[]))
98+
Filter: (_timescaledb_functions.bloom1_contains_any_hashes(regress-test-bloom_7035_a_b, TEST-HASHES::bigint[]) AND _timescaledb_functions.bloom1_contains_any_hashes(regress-test-bloom_a, TEST-HASHES::bigint[]) AND _timescaledb_functions.bloom1_contains_any_hashes(regress-test-bloom_b, TEST-HASHES::bigint[]))
9999
Rows Removed by Filter: 3
100100
-> Sort (actual rows=0.00 loops=1)
101101
Sort Key: _hyper_1_3_chunk.ts, _hyper_1_3_chunk.seg

tsl/test/expected/compress_compbloom_manual_config.out

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ ORDER BY 1,2,3,4;
5555
-> Custom Scan (ColumnarScan) on _hyper_1_1_chunk (actual rows=0.00 loops=1)
5656
Vectorized Filter: ((a = 1) AND (b = 2))
5757
-> Seq Scan on compress_hyper_2_4_chunk (actual rows=0.00 loops=1)
58-
Filter: _timescaledb_functions.bloom1_contains_any_hashes(regress-test-bloom_a_b, TEST-HASHES::bigint[])
58+
Filter: _timescaledb_functions.bloom1_contains_any_hashes(regress-test-bloom_7035_a_b, TEST-HASHES::bigint[])
5959
Rows Removed by Filter: 2
6060
-> Sort (actual rows=0.00 loops=1)
6161
Sort Key: _hyper_1_2_chunk.ts, _hyper_1_2_chunk.c
@@ -175,7 +175,7 @@ ORDER BY 1,2,3,4;
175175
-> Custom Scan (ColumnarScan) on _hyper_1_1_chunk (actual rows=0.00 loops=1)
176176
Vectorized Filter: ((a = 1) AND (b = 2))
177177
-> Seq Scan on compress_hyper_2_4_chunk (actual rows=0.00 loops=1)
178-
Filter: _timescaledb_functions.bloom1_contains_any_hashes(regress-test-bloom_a_b, TEST-HASHES::bigint[])
178+
Filter: _timescaledb_functions.bloom1_contains_any_hashes(regress-test-bloom_7035_a_b, TEST-HASHES::bigint[])
179179
Rows Removed by Filter: 2
180180
-> Sort (actual rows=0.00 loops=1)
181181
Sort Key: _hyper_1_2_chunk.ts, _hyper_1_2_chunk.c
@@ -212,7 +212,7 @@ ORDER BY 1,2,3,4;
212212
Vectorized Filter: ((a = 1) AND (c = 2))
213213
Rows Removed by Filter: 2328
214214
-> Seq Scan on compress_hyper_2_5_chunk (actual rows=3.00 loops=1)
215-
Filter: _timescaledb_functions.bloom1_contains_any_hashes(regress-test-bloom_a_c, TEST-HASHES::bigint[])
215+
Filter: _timescaledb_functions.bloom1_contains_any_hashes(regress-test-bloom_e8f0_a_c, TEST-HASHES::bigint[])
216216
-> Sort (actual rows=27.00 loops=1)
217217
Sort Key: _hyper_1_3_chunk.ts, _hyper_1_3_chunk.b
218218
Sort Method: quicksort
@@ -248,6 +248,6 @@ ORDER BY 1,2,3,4;
248248
Vectorized Filter: ((b = 1) AND (c = 2))
249249
Rows Removed by Filter: 846
250250
-> Seq Scan on compress_hyper_2_6_chunk (actual rows=1.00 loops=1)
251-
Filter: _timescaledb_functions.bloom1_contains_any_hashes(regress-test-bloom_b_c, TEST-HASHES::bigint[])
251+
Filter: _timescaledb_functions.bloom1_contains_any_hashes(regress-test-bloom_7ba1_b_c, TEST-HASHES::bigint[])
252252

253253
DROP TABLE IF EXISTS mixed_avail_manual CASCADE;

0 commit comments

Comments
 (0)