diff --git a/.unreleased/pr_9736 b/.unreleased/pr_9736 new file mode 100644 index 00000000000..0d800edcb90 --- /dev/null +++ b/.unreleased/pr_9736 @@ -0,0 +1 @@ +Fixes: #9736 Do logical sparse index comparison diff --git a/src/ts_catalog/compression_settings.c b/src/ts_catalog/compression_settings.c index b793f7952a8..03e9e5c1f18 100644 --- a/src/ts_catalog/compression_settings.c +++ b/src/ts_catalog/compression_settings.c @@ -27,6 +27,9 @@ static ScanTupleResult compression_settings_tuple_update(TupleInfo *ti, void *da static HeapTuple compression_settings_formdata_make_tuple(const FormData_compression_settings *fd, TupleDesc desc); static Bitmapset *resolve_columns_to_attnos(List *column_names, Oid relid); +static bool sparse_index_values_equal(List *left, List *right); +static bool sparse_index_object_equal(SparseIndexSettingsObject *left, + SparseIndexSettingsObject *right); /* * Compare two compression settings for equality @@ -38,7 +41,7 @@ ts_compression_settings_equal(const CompressionSettings *left, const Compression ts_array_equal(left->fd.orderby, right->fd.orderby) && ts_array_equal(left->fd.orderby_desc, right->fd.orderby_desc) && ts_array_equal(left->fd.orderby_nullsfirst, right->fd.orderby_nullsfirst) && - ts_jsonb_equal(left->fd.index, right->fd.index); + ts_sparse_index_equal(left->fd.index, right->fd.index); } /* @@ -62,7 +65,125 @@ ts_compression_settings_equal_with_defaults(const CompressionSettings *ht, ts_array_equal(ht->fd.orderby_desc, chunk->fd.orderby_desc)) && (ht->fd.orderby_nullsfirst == NULL || ts_array_equal(ht->fd.orderby_nullsfirst, chunk->fd.orderby_nullsfirst)) && - (ht->fd.index == NULL || ts_jsonb_equal(ht->fd.index, chunk->fd.index)); + (ht->fd.index == NULL || ts_sparse_index_equal(ht->fd.index, chunk->fd.index)); +} + +/* + * Compare two string value lists for equality (order-sensitive). + */ +static bool +sparse_index_values_equal(List *left, List *right) +{ + if (list_length(left) != list_length(right)) + { + return false; + } + + ListCell *lc_left, *lc_right; + forboth (lc_left, left, lc_right, right) + { + if (strcmp((const char *) lfirst(lc_left), (const char *) lfirst(lc_right)) != 0) + { + return false; + } + } + return true; +} + +/* + * Compare two sparse index objects for equality. + * Two objects are equal if they have the same pairs with the same values. + */ +static bool +sparse_index_object_equal(SparseIndexSettingsObject *left, SparseIndexSettingsObject *right) +{ + if (list_length(left->pairs) != list_length(right->pairs)) + { + return false; + } + + foreach_ptr(SparseIndexSettingsPair, lpair, left->pairs) + { + bool found = false; + foreach_ptr(SparseIndexSettingsPair, rpair, right->pairs) + { + if (strcmp(lpair->key, rpair->key) == 0) + { + if (!sparse_index_values_equal(lpair->values, rpair->values)) + { + return false; + } + found = true; + break; + } + } + if (!found) + { + return false; + } + } + return true; +} + +/* + * Compare two sparse index JSONB settings for equality, independent of + * the order of objects in the array. Each object is matched by its + * key-value pairs (type, column, source). + */ +bool +ts_sparse_index_equal(const Jsonb *left, const Jsonb *right) +{ + if (left == right) + { + return true; + } + if (left == NULL || right == NULL) + { + return false; + } + + SparseIndexSettings *left_settings = ts_convert_to_sparse_index_settings((Jsonb *) left); + SparseIndexSettings *right_settings = ts_convert_to_sparse_index_settings((Jsonb *) right); + + int n_left = list_length(left_settings->objects); + int n_right = list_length(right_settings->objects); + + if (n_left != n_right) + { + ts_free_sparse_index_settings(left_settings); + ts_free_sparse_index_settings(right_settings); + return false; + } + + /* for tracking */ + bool *already_found = palloc0(sizeof(bool) * n_left); + bool equal = true; + + foreach_ptr(SparseIndexSettingsObject, lobj, left_settings->objects) + { + int ri = 0; + bool found = false; + foreach_ptr(SparseIndexSettingsObject, robj, right_settings->objects) + { + if (!already_found[ri] && sparse_index_object_equal(lobj, robj)) + { + already_found[ri] = true; + found = true; + break; + } + ri++; + } + if (!found) + { + equal = false; + break; + } + } + + pfree(already_found); + ts_free_sparse_index_settings(left_settings); + ts_free_sparse_index_settings(right_settings); + return equal; } CompressionSettings * diff --git a/src/ts_catalog/compression_settings.h b/src/ts_catalog/compression_settings.h index 9b6852ead32..fca8452d8d4 100644 --- a/src/ts_catalog/compression_settings.h +++ b/src/ts_catalog/compression_settings.h @@ -151,6 +151,7 @@ TSDLLEXPORT bool ts_compression_settings_equal(const CompressionSettings *left, const CompressionSettings *right); TSDLLEXPORT bool ts_compression_settings_equal_with_defaults(const CompressionSettings *ht, const CompressionSettings *chunk); +TSDLLEXPORT bool ts_sparse_index_equal(const Jsonb *left, const Jsonb *right); TSDLLEXPORT int ts_compression_settings_update(CompressionSettings *settings); TSDLLEXPORT void ts_compression_settings_rename_column_cascade(Oid parent_relid, const char *old, diff --git a/test/src/test_compression_settings.c b/test/src/test_compression_settings.c index c31fe3cc01c..200a391ea1f 100644 --- a/test/src/test_compression_settings.c +++ b/test/src/test_compression_settings.c @@ -337,10 +337,143 @@ test_convert_to_sparse_index_settings() } } +static void +test_sparse_index_equal() +{ + /* Both NULL — equal */ + TestAssertBoolEq(ts_sparse_index_equal(NULL, NULL), true); + + /* One NULL, one non-NULL — not equal */ + { + Jsonb *jb = + cstring_to_jsonb("[{\"type\": \"bloom\", \"column\": \"x\", \"source\": \"config\"}]"); + TestAssertBoolEq(ts_sparse_index_equal(NULL, jb), false); + TestAssertBoolEq(ts_sparse_index_equal(jb, NULL), false); + pfree(jb); + } + + /* Identical JSONB — equal */ + { + Jsonb *jb = cstring_to_jsonb( + "[{\"type\": \"bloom\", \"column\": \"x\", \"source\": \"config\"}, " + "{\"type\": \"minmax\", \"column\": \"ts\", \"source\": \"orderby\"}]"); + TestAssertBoolEq(ts_sparse_index_equal(jb, jb), true); + pfree(jb); + } + + /* Same objects, different array order — equal */ + { + Jsonb *a = cstring_to_jsonb( + "[{\"type\": \"bloom\", \"column\": \"x\", \"source\": \"config\"}, " + "{\"type\": \"minmax\", \"column\": \"ts\", \"source\": \"orderby\"}]"); + Jsonb *b = cstring_to_jsonb( + "[{\"type\": \"minmax\", \"column\": \"ts\", \"source\": \"orderby\"}, " + "{\"type\": \"bloom\", \"column\": \"x\", \"source\": \"config\"}]"); + TestAssertBoolEq(ts_sparse_index_equal(a, b), true); + pfree(a); + pfree(b); + } + + /* Different column value — not equal */ + { + Jsonb *a = + cstring_to_jsonb("[{\"type\": \"bloom\", \"column\": \"x\", \"source\": \"config\"}]"); + Jsonb *b = + cstring_to_jsonb("[{\"type\": \"bloom\", \"column\": \"y\", \"source\": \"config\"}]"); + TestAssertBoolEq(ts_sparse_index_equal(a, b), false); + pfree(a); + pfree(b); + } + + /* Different number of objects — not equal */ + { + Jsonb *a = + cstring_to_jsonb("[{\"type\": \"bloom\", \"column\": \"x\", \"source\": \"config\"}]"); + Jsonb *b = cstring_to_jsonb( + "[{\"type\": \"bloom\", \"column\": \"x\", \"source\": \"config\"}, " + "{\"type\": \"minmax\", \"column\": \"ts\", \"source\": \"orderby\"}]"); + TestAssertBoolEq(ts_sparse_index_equal(a, b), false); + pfree(a); + pfree(b); + } + + /* Composite bloom columns, same order — equal */ + { + Jsonb *a = cstring_to_jsonb( + "[{\"type\": \"bloom\", \"column\": [\"a\", \"b\"], \"source\": \"config\"}]"); + Jsonb *b = cstring_to_jsonb( + "[{\"type\": \"bloom\", \"column\": [\"a\", \"b\"], \"source\": \"config\"}]"); + TestAssertBoolEq(ts_sparse_index_equal(a, b), true); + pfree(a); + pfree(b); + } + + /* Composite bloom columns, different column order — not equal. */ + /* Should not be possible but keep this test to flag if something breaks this logic */ + { + Jsonb *a = cstring_to_jsonb( + "[{\"type\": \"bloom\", \"column\": [\"a\", \"b\"], \"source\": \"config\"}]"); + Jsonb *b = cstring_to_jsonb( + "[{\"type\": \"bloom\", \"column\": [\"b\", \"a\"], \"source\": \"config\"}]"); + TestAssertBoolEq(ts_sparse_index_equal(a, b), false); + pfree(a); + pfree(b); + } + + /* Different type — not equal */ + { + Jsonb *a = + cstring_to_jsonb("[{\"type\": \"bloom\", \"column\": \"x\", \"source\": \"config\"}]"); + Jsonb *b = + cstring_to_jsonb("[{\"type\": \"minmax\", \"column\": \"x\", \"source\": \"config\"}]"); + TestAssertBoolEq(ts_sparse_index_equal(a, b), false); + pfree(a); + pfree(b); + } + + /* Extra keys — not equal */ + { + Jsonb *a = + cstring_to_jsonb("[{\"type\": \"bloom\", \"column\": \"x\", \"source\": \"config\"}]"); + Jsonb *b = cstring_to_jsonb( + "[{\"type\": \"bloom\", \"column\": \"x\", \"source\": \"config\",\"foo\":\"bar\"}]"); + TestAssertBoolEq(ts_sparse_index_equal(a, b), false); + pfree(a); + pfree(b); + } + + /* Different source — not equal */ + { + Jsonb *a = cstring_to_jsonb( + "[{\"type\": \"minmax\", \"column\": \"ts\", \"source\": \"config\"}]"); + Jsonb *b = cstring_to_jsonb( + "[{\"type\": \"minmax\", \"column\": \"ts\", \"source\": \"orderby\"}]"); + TestAssertBoolEq(ts_sparse_index_equal(a, b), false); + pfree(a); + pfree(b); + } + + /* Three objects shuffled — equal */ + { + Jsonb *a = cstring_to_jsonb( + "[{\"type\": \"bloom\", \"column\": \"a\", \"source\": \"config\"}, " + "{\"type\": \"bloom\", \"column\": \"b\", \"source\": \"config\"}, " + "{\"type\": \"minmax\", \"column\": \"ts\", \"source\": \"orderby\"}]"); + Jsonb *b = cstring_to_jsonb( + "[{\"type\": \"minmax\", \"column\": \"ts\", \"source\": \"orderby\"}, " + "{\"type\": \"bloom\", \"column\": \"b\", \"source\": \"config\"}, " + "{\"type\": \"bloom\", \"column\": \"a\", \"source\": \"config\"}]"); + TestAssertBoolEq(ts_sparse_index_equal(a, b), true); + pfree(a); + pfree(b); + } +} + TS_TEST_FN(ts_test_compression_settings) { test_alter_table_rename_column_effect_jsonb(); test_alter_table_drop_column_effect_jsonb(); test_convert_to_sparse_index_settings(); + test_sparse_index_equal(); PG_RETURN_VOID(); } diff --git a/tsl/test/expected/merge_chunks.out b/tsl/test/expected/merge_chunks.out index 6e87b619237..0c73a689540 100644 --- a/tsl/test/expected/merge_chunks.out +++ b/tsl/test/expected/merge_chunks.out @@ -1056,3 +1056,102 @@ FROM concurrent_compressed_merge, 2 | 1 DROP TABLE concurrent_compressed_merge; +-- Test merging compressed chunks whose sparse indexes are logically the same +-- but the JSONB entries mismatch +CREATE TABLE merge_sparse_order( + time timestamptz NOT NULL, + device int, + temp float8, + val int +); +SELECT create_hypertable('merge_sparse_order', 'time', chunk_time_interval => INTERVAL '1 day'); + create_hypertable +---------------------------------- + (11,public,merge_sparse_order,t) + +INSERT INTO merge_sparse_order +SELECT t, (i % 10) + 1, random() * 100, (i * 7) % 13 +FROM generate_series('2024-01-01 2:00'::timestamptz, '2024-01-01 23:59', '1 minute') t, + generate_series(1, 5) i; +INSERT INTO merge_sparse_order +SELECT t, (i % 10) + 1, random() * 100, (i * 7) % 13 +FROM generate_series('2024-01-02 2:00'::timestamptz, '2024-01-02 23:59', '1 minute') t, + generate_series(1, 5) i; +SELECT format('%I.%I', schema_name, table_name) AS mso_c1 + FROM _timescaledb_catalog.chunk + WHERE hypertable_id = (SELECT id FROM _timescaledb_catalog.hypertable + WHERE table_name = 'merge_sparse_order') + ORDER BY id LIMIT 1 \gset +SELECT format('%I.%I', schema_name, table_name) AS mso_c2 + FROM _timescaledb_catalog.chunk + WHERE hypertable_id = (SELECT id FROM _timescaledb_catalog.hypertable + WHERE table_name = 'merge_sparse_order') + ORDER BY id OFFSET 1 LIMIT 1 \gset +ALTER TABLE merge_sparse_order SET ( + timescaledb.compress, + timescaledb.compress_orderby = 'time', + timescaledb.compress_index = 'bloom("device"), minmax("temp"), bloom("device","val")' +); +SELECT compress_chunk(:'mso_c1'); + compress_chunk +------------------------------------------ + _timescaledb_internal._hyper_11_32_chunk + +ALTER TABLE merge_sparse_order SET ( + timescaledb.compress, + timescaledb.compress_orderby = 'time', + timescaledb.compress_index = 'minmax("temp"), bloom("device")' +); +NOTICE: updated compression settings will only apply to future compressions +SELECT compress_chunk(:'mso_c2'); + compress_chunk +------------------------------------------ + _timescaledb_internal._hyper_11_33_chunk + +-- have different sparse indexes +SELECT relid, index FROM _timescaledb_catalog.compression_settings ORDER BY relid::text COLLATE "C"; + relid | index +------------------------------------------+------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ + _timescaledb_internal._hyper_11_32_chunk | [{"type": "bloom", "column": "device", "source": "config"}, {"type": "minmax", "column": "temp", "source": "config"}, {"type": "bloom", "column": ["device", "val"], "source": "config"}, {"type": "minmax", "column": "time", "source": "orderby"}] + _timescaledb_internal._hyper_11_33_chunk | [{"type": "minmax", "column": "temp", "source": "config"}, {"type": "bloom", "column": "device", "source": "config"}, {"type": "minmax", "column": "time", "source": "orderby"}] + _timescaledb_internal._hyper_1_1_chunk | [{"type": "minmax", "column": "time", "source": "orderby"}] + merge_sparse_order | [{"type": "minmax", "column": "temp", "source": "config"}, {"type": "bloom", "column": "device", "source": "config"}, {"type": "minmax", "column": "time", "source": "orderby"}] + mergeme | [{"type": "minmax", "column": "time", "source": "orderby"}] + +-- Merge should not succeed +\set ON_ERROR_STOP 0 +CALL merge_chunks(:'mso_c1'::regclass, :'mso_c2'::regclass); +ERROR: cannot merge compressed chunks with different compression settings +\set ON_ERROR_STOP 1 +ALTER TABLE merge_sparse_order SET ( + timescaledb.compress, + timescaledb.compress_orderby = 'time', + timescaledb.compress_index = 'bloom("val","device"), minmax("temp"), bloom("device")' +); +NOTICE: updated compression settings will only apply to future compressions +SELECT compress_chunk(decompress_chunk(:'mso_c2')); + compress_chunk +------------------------------------------ + _timescaledb_internal._hyper_11_33_chunk + +-- have different JSONB but same logical sparse indexes +SELECT relid, index FROM _timescaledb_catalog.compression_settings ORDER BY relid::text COLLATE "C"; + relid | index +------------------------------------------+------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ + _timescaledb_internal._hyper_11_32_chunk | [{"type": "bloom", "column": "device", "source": "config"}, {"type": "minmax", "column": "temp", "source": "config"}, {"type": "bloom", "column": ["device", "val"], "source": "config"}, {"type": "minmax", "column": "time", "source": "orderby"}] + _timescaledb_internal._hyper_11_33_chunk | [{"type": "bloom", "column": ["device", "val"], "source": "config"}, {"type": "minmax", "column": "temp", "source": "config"}, {"type": "bloom", "column": "device", "source": "config"}, {"type": "minmax", "column": "time", "source": "orderby"}] + _timescaledb_internal._hyper_1_1_chunk | [{"type": "minmax", "column": "time", "source": "orderby"}] + merge_sparse_order | [{"type": "bloom", "column": ["device", "val"], "source": "config"}, {"type": "minmax", "column": "temp", "source": "config"}, {"type": "bloom", "column": "device", "source": "config"}, {"type": "minmax", "column": "time", "source": "orderby"}] + mergeme | [{"type": "minmax", "column": "time", "source": "orderby"}] + +-- Merge should succeed +CALL merge_chunks(:'mso_c1'::regclass, :'mso_c2'::regclass); +SELECT relid, index FROM _timescaledb_catalog.compression_settings ORDER BY relid::text COLLATE "C"; + relid | index +------------------------------------------+------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ + _timescaledb_internal._hyper_11_32_chunk | [{"type": "bloom", "column": "device", "source": "config"}, {"type": "minmax", "column": "temp", "source": "config"}, {"type": "bloom", "column": ["device", "val"], "source": "config"}, {"type": "minmax", "column": "time", "source": "orderby"}] + _timescaledb_internal._hyper_1_1_chunk | [{"type": "minmax", "column": "time", "source": "orderby"}] + merge_sparse_order | [{"type": "bloom", "column": ["device", "val"], "source": "config"}, {"type": "minmax", "column": "temp", "source": "config"}, {"type": "bloom", "column": "device", "source": "config"}, {"type": "minmax", "column": "time", "source": "orderby"}] + mergeme | [{"type": "minmax", "column": "time", "source": "orderby"}] + +DROP TABLE merge_sparse_order; diff --git a/tsl/test/sql/merge_chunks.sql b/tsl/test/sql/merge_chunks.sql index 91240f10cbc..788e99cda60 100644 --- a/tsl/test/sql/merge_chunks.sql +++ b/tsl/test/sql/merge_chunks.sql @@ -589,3 +589,73 @@ FROM concurrent_compressed_merge, show_chunks('concurrent_compressed_merge'); DROP TABLE concurrent_compressed_merge; + +-- Test merging compressed chunks whose sparse indexes are logically the same +-- but the JSONB entries mismatch +CREATE TABLE merge_sparse_order( + time timestamptz NOT NULL, + device int, + temp float8, + val int +); +SELECT create_hypertable('merge_sparse_order', 'time', chunk_time_interval => INTERVAL '1 day'); + +INSERT INTO merge_sparse_order +SELECT t, (i % 10) + 1, random() * 100, (i * 7) % 13 +FROM generate_series('2024-01-01 2:00'::timestamptz, '2024-01-01 23:59', '1 minute') t, + generate_series(1, 5) i; +INSERT INTO merge_sparse_order +SELECT t, (i % 10) + 1, random() * 100, (i * 7) % 13 +FROM generate_series('2024-01-02 2:00'::timestamptz, '2024-01-02 23:59', '1 minute') t, + generate_series(1, 5) i; + +SELECT format('%I.%I', schema_name, table_name) AS mso_c1 + FROM _timescaledb_catalog.chunk + WHERE hypertable_id = (SELECT id FROM _timescaledb_catalog.hypertable + WHERE table_name = 'merge_sparse_order') + ORDER BY id LIMIT 1 \gset +SELECT format('%I.%I', schema_name, table_name) AS mso_c2 + FROM _timescaledb_catalog.chunk + WHERE hypertable_id = (SELECT id FROM _timescaledb_catalog.hypertable + WHERE table_name = 'merge_sparse_order') + ORDER BY id OFFSET 1 LIMIT 1 \gset + +ALTER TABLE merge_sparse_order SET ( + timescaledb.compress, + timescaledb.compress_orderby = 'time', + timescaledb.compress_index = 'bloom("device"), minmax("temp"), bloom("device","val")' +); +SELECT compress_chunk(:'mso_c1'); + +ALTER TABLE merge_sparse_order SET ( + timescaledb.compress, + timescaledb.compress_orderby = 'time', + timescaledb.compress_index = 'minmax("temp"), bloom("device")' +); +SELECT compress_chunk(:'mso_c2'); + +-- have different sparse indexes +SELECT relid, index FROM _timescaledb_catalog.compression_settings ORDER BY relid::text COLLATE "C"; + +-- Merge should not succeed +\set ON_ERROR_STOP 0 +CALL merge_chunks(:'mso_c1'::regclass, :'mso_c2'::regclass); +\set ON_ERROR_STOP 1 + +ALTER TABLE merge_sparse_order SET ( + timescaledb.compress, + timescaledb.compress_orderby = 'time', + timescaledb.compress_index = 'bloom("val","device"), minmax("temp"), bloom("device")' +); + +SELECT compress_chunk(decompress_chunk(:'mso_c2')); + +-- have different JSONB but same logical sparse indexes +SELECT relid, index FROM _timescaledb_catalog.compression_settings ORDER BY relid::text COLLATE "C"; + +-- Merge should succeed +CALL merge_chunks(:'mso_c1'::regclass, :'mso_c2'::regclass); + +SELECT relid, index FROM _timescaledb_catalog.compression_settings ORDER BY relid::text COLLATE "C"; + +DROP TABLE merge_sparse_order;