Skip to content

Commit 7f6750d

Browse files
Poroma-BanerjeePoroma-Banerjee
committed
Do logical sparse index comparison
Sparse index JSONB arrays can have objects in different order while being logically identical, e.g. after ALTER TABLE changes the compress_index declaration order. Replace ts_jsonb_equal with a new ts_sparse_index_equal that parses both sides and matches objects as an unordered set.
1 parent 44f54bd commit 7f6750d

6 files changed

Lines changed: 400 additions & 2 deletions

File tree

‎.unreleased/pr_9736‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fixes: #9736 Do logical sparse index comparison

‎src/ts_catalog/compression_settings.c‎

Lines changed: 123 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,9 @@ static ScanTupleResult compression_settings_tuple_update(TupleInfo *ti, void *da
2727
static HeapTuple compression_settings_formdata_make_tuple(const FormData_compression_settings *fd,
2828
TupleDesc desc);
2929
static Bitmapset *resolve_columns_to_attnos(List *column_names, Oid relid);
30+
static bool sparse_index_values_equal(List *left, List *right);
31+
static bool sparse_index_object_equal(SparseIndexSettingsObject *left,
32+
SparseIndexSettingsObject *right);
3033

3134
/*
3235
* Compare two compression settings for equality
@@ -38,7 +41,7 @@ ts_compression_settings_equal(const CompressionSettings *left, const Compression
3841
ts_array_equal(left->fd.orderby, right->fd.orderby) &&
3942
ts_array_equal(left->fd.orderby_desc, right->fd.orderby_desc) &&
4043
ts_array_equal(left->fd.orderby_nullsfirst, right->fd.orderby_nullsfirst) &&
41-
ts_jsonb_equal(left->fd.index, right->fd.index);
44+
ts_sparse_index_equal(left->fd.index, right->fd.index);
4245
}
4346

4447
/*
@@ -62,7 +65,125 @@ ts_compression_settings_equal_with_defaults(const CompressionSettings *ht,
6265
ts_array_equal(ht->fd.orderby_desc, chunk->fd.orderby_desc)) &&
6366
(ht->fd.orderby_nullsfirst == NULL ||
6467
ts_array_equal(ht->fd.orderby_nullsfirst, chunk->fd.orderby_nullsfirst)) &&
65-
(ht->fd.index == NULL || ts_jsonb_equal(ht->fd.index, chunk->fd.index));
68+
(ht->fd.index == NULL || ts_sparse_index_equal(ht->fd.index, chunk->fd.index));
69+
}
70+
71+
/*
72+
* Compare two string value lists for equality (order-sensitive).
73+
*/
74+
static bool
75+
sparse_index_values_equal(List *left, List *right)
76+
{
77+
if (list_length(left) != list_length(right))
78+
{
79+
return false;
80+
}
81+
82+
ListCell *lc_left, *lc_right;
83+
forboth (lc_left, left, lc_right, right)
84+
{
85+
if (strcmp((const char *) lfirst(lc_left), (const char *) lfirst(lc_right)) != 0)
86+
{
87+
return false;
88+
}
89+
}
90+
return true;
91+
}
92+
93+
/*
94+
* Compare two sparse index objects for equality.
95+
* Two objects are equal if they have the same pairs with the same values.
96+
*/
97+
static bool
98+
sparse_index_object_equal(SparseIndexSettingsObject *left, SparseIndexSettingsObject *right)
99+
{
100+
if (list_length(left->pairs) != list_length(right->pairs))
101+
{
102+
return false;
103+
}
104+
105+
foreach_ptr(SparseIndexSettingsPair, lpair, left->pairs)
106+
{
107+
bool found = false;
108+
foreach_ptr(SparseIndexSettingsPair, rpair, right->pairs)
109+
{
110+
if (strcmp(lpair->key, rpair->key) == 0)
111+
{
112+
if (!sparse_index_values_equal(lpair->values, rpair->values))
113+
{
114+
return false;
115+
}
116+
found = true;
117+
break;
118+
}
119+
}
120+
if (!found)
121+
{
122+
return false;
123+
}
124+
}
125+
return true;
126+
}
127+
128+
/*
129+
* Compare two sparse index JSONB settings for equality, independent of
130+
* the order of objects in the array. Each object is matched by its
131+
* key-value pairs (type, column, source).
132+
*/
133+
bool
134+
ts_sparse_index_equal(const Jsonb *left, const Jsonb *right)
135+
{
136+
if (left == right)
137+
{
138+
return true;
139+
}
140+
if (left == NULL || right == NULL)
141+
{
142+
return false;
143+
}
144+
145+
SparseIndexSettings *left_settings = ts_convert_to_sparse_index_settings((Jsonb *) left);
146+
SparseIndexSettings *right_settings = ts_convert_to_sparse_index_settings((Jsonb *) right);
147+
148+
int n_left = list_length(left_settings->objects);
149+
int n_right = list_length(right_settings->objects);
150+
151+
if (n_left != n_right)
152+
{
153+
ts_free_sparse_index_settings(left_settings);
154+
ts_free_sparse_index_settings(right_settings);
155+
return false;
156+
}
157+
158+
/* for tracking */
159+
bool *already_found = palloc0(sizeof(bool) * n_left);
160+
bool equal = true;
161+
162+
foreach_ptr(SparseIndexSettingsObject, lobj, left_settings->objects)
163+
{
164+
int ri = 0;
165+
bool found = false;
166+
foreach_ptr(SparseIndexSettingsObject, robj, right_settings->objects)
167+
{
168+
if (!already_found[ri] && sparse_index_object_equal(lobj, robj))
169+
{
170+
already_found[ri] = true;
171+
found = true;
172+
break;
173+
}
174+
ri++;
175+
}
176+
if (!found)
177+
{
178+
equal = false;
179+
break;
180+
}
181+
}
182+
183+
pfree(already_found);
184+
ts_free_sparse_index_settings(left_settings);
185+
ts_free_sparse_index_settings(right_settings);
186+
return equal;
66187
}
67188

68189
CompressionSettings *

‎src/ts_catalog/compression_settings.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,7 @@ TSDLLEXPORT bool ts_compression_settings_equal(const CompressionSettings *left,
151151
const CompressionSettings *right);
152152
TSDLLEXPORT bool ts_compression_settings_equal_with_defaults(const CompressionSettings *ht,
153153
const CompressionSettings *chunk);
154+
TSDLLEXPORT bool ts_sparse_index_equal(const Jsonb *left, const Jsonb *right);
154155

155156
TSDLLEXPORT int ts_compression_settings_update(CompressionSettings *settings);
156157
TSDLLEXPORT void ts_compression_settings_rename_column_cascade(Oid parent_relid, const char *old,

‎test/src/test_compression_settings.c‎

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -337,10 +337,132 @@ test_convert_to_sparse_index_settings()
337337
}
338338
}
339339

340+
static void
341+
test_sparse_index_equal()
342+
{
343+
/* Both NULL — equal */
344+
TestAssertBoolEq(ts_sparse_index_equal(NULL, NULL), true);
345+
346+
/* One NULL, one non-NULL — not equal */
347+
{
348+
Jsonb *jb =
349+
cstring_to_jsonb("[{\"type\": \"bloom\", \"column\": \"x\", \"source\": \"config\"}]");
350+
TestAssertBoolEq(ts_sparse_index_equal(NULL, jb), false);
351+
TestAssertBoolEq(ts_sparse_index_equal(jb, NULL), false);
352+
pfree(jb);
353+
}
354+
355+
/* Identical JSONB — equal */
356+
{
357+
Jsonb *jb = cstring_to_jsonb(
358+
"[{\"type\": \"bloom\", \"column\": \"x\", \"source\": \"config\"}, "
359+
"{\"type\": \"minmax\", \"column\": \"ts\", \"source\": \"orderby\"}]");
360+
TestAssertBoolEq(ts_sparse_index_equal(jb, jb), true);
361+
pfree(jb);
362+
}
363+
364+
/* Same objects, different array order — equal */
365+
{
366+
Jsonb *a = cstring_to_jsonb(
367+
"[{\"type\": \"bloom\", \"column\": \"x\", \"source\": \"config\"}, "
368+
"{\"type\": \"minmax\", \"column\": \"ts\", \"source\": \"orderby\"}]");
369+
Jsonb *b = cstring_to_jsonb(
370+
"[{\"type\": \"minmax\", \"column\": \"ts\", \"source\": \"orderby\"}, "
371+
"{\"type\": \"bloom\", \"column\": \"x\", \"source\": \"config\"}]");
372+
TestAssertBoolEq(ts_sparse_index_equal(a, b), true);
373+
pfree(a);
374+
pfree(b);
375+
}
376+
377+
/* Different column value — not equal */
378+
{
379+
Jsonb *a =
380+
cstring_to_jsonb("[{\"type\": \"bloom\", \"column\": \"x\", \"source\": \"config\"}]");
381+
Jsonb *b =
382+
cstring_to_jsonb("[{\"type\": \"bloom\", \"column\": \"y\", \"source\": \"config\"}]");
383+
TestAssertBoolEq(ts_sparse_index_equal(a, b), false);
384+
pfree(a);
385+
pfree(b);
386+
}
387+
388+
/* Different number of objects — not equal */
389+
{
390+
Jsonb *a =
391+
cstring_to_jsonb("[{\"type\": \"bloom\", \"column\": \"x\", \"source\": \"config\"}]");
392+
Jsonb *b = cstring_to_jsonb(
393+
"[{\"type\": \"bloom\", \"column\": \"x\", \"source\": \"config\"}, "
394+
"{\"type\": \"minmax\", \"column\": \"ts\", \"source\": \"orderby\"}]");
395+
TestAssertBoolEq(ts_sparse_index_equal(a, b), false);
396+
pfree(a);
397+
pfree(b);
398+
}
399+
400+
/* Composite bloom columns, same order — equal */
401+
{
402+
Jsonb *a = cstring_to_jsonb(
403+
"[{\"type\": \"bloom\", \"column\": [\"a\", \"b\"], \"source\": \"config\"}]");
404+
Jsonb *b = cstring_to_jsonb(
405+
"[{\"type\": \"bloom\", \"column\": [\"a\", \"b\"], \"source\": \"config\"}]");
406+
TestAssertBoolEq(ts_sparse_index_equal(a, b), true);
407+
pfree(a);
408+
pfree(b);
409+
}
410+
411+
/* Composite bloom columns, different column order — not equal. */
412+
/* Should not be possible but keep this test to flag if something breaks this logic */
413+
{
414+
Jsonb *a = cstring_to_jsonb(
415+
"[{\"type\": \"bloom\", \"column\": [\"a\", \"b\"], \"source\": \"config\"}]");
416+
Jsonb *b = cstring_to_jsonb(
417+
"[{\"type\": \"bloom\", \"column\": [\"b\", \"a\"], \"source\": \"config\"}]");
418+
TestAssertBoolEq(ts_sparse_index_equal(a, b), false);
419+
pfree(a);
420+
pfree(b);
421+
}
422+
423+
/* Different type — not equal */
424+
{
425+
Jsonb *a =
426+
cstring_to_jsonb("[{\"type\": \"bloom\", \"column\": \"x\", \"source\": \"config\"}]");
427+
Jsonb *b =
428+
cstring_to_jsonb("[{\"type\": \"minmax\", \"column\": \"x\", \"source\": \"config\"}]");
429+
TestAssertBoolEq(ts_sparse_index_equal(a, b), false);
430+
pfree(a);
431+
pfree(b);
432+
}
433+
434+
/* Different source — not equal */
435+
{
436+
Jsonb *a = cstring_to_jsonb(
437+
"[{\"type\": \"minmax\", \"column\": \"ts\", \"source\": \"config\"}]");
438+
Jsonb *b = cstring_to_jsonb(
439+
"[{\"type\": \"minmax\", \"column\": \"ts\", \"source\": \"orderby\"}]");
440+
TestAssertBoolEq(ts_sparse_index_equal(a, b), false);
441+
pfree(a);
442+
pfree(b);
443+
}
444+
445+
/* Three objects shuffled — equal */
446+
{
447+
Jsonb *a = cstring_to_jsonb(
448+
"[{\"type\": \"bloom\", \"column\": \"a\", \"source\": \"config\"}, "
449+
"{\"type\": \"bloom\", \"column\": \"b\", \"source\": \"config\"}, "
450+
"{\"type\": \"minmax\", \"column\": \"ts\", \"source\": \"orderby\"}]");
451+
Jsonb *b = cstring_to_jsonb(
452+
"[{\"type\": \"minmax\", \"column\": \"ts\", \"source\": \"orderby\"}, "
453+
"{\"type\": \"bloom\", \"column\": \"b\", \"source\": \"config\"}, "
454+
"{\"type\": \"bloom\", \"column\": \"a\", \"source\": \"config\"}]");
455+
TestAssertBoolEq(ts_sparse_index_equal(a, b), true);
456+
pfree(a);
457+
pfree(b);
458+
}
459+
}
460+
340461
TS_TEST_FN(ts_test_compression_settings)
341462
{
342463
test_alter_table_rename_column_effect_jsonb();
343464
test_alter_table_drop_column_effect_jsonb();
344465
test_convert_to_sparse_index_settings();
466+
test_sparse_index_equal();
345467
PG_RETURN_VOID();
346468
}

‎tsl/test/expected/merge_chunks.out‎

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1056,3 +1056,102 @@ FROM concurrent_compressed_merge,
10561056
2 | 1
10571057

10581058
DROP TABLE concurrent_compressed_merge;
1059+
-- Test merging compressed chunks whose sparse indexes are logically the same
1060+
-- but the JSONB entries mismatch
1061+
CREATE TABLE merge_sparse_order(
1062+
time timestamptz NOT NULL,
1063+
device int,
1064+
temp float8,
1065+
val int
1066+
);
1067+
SELECT create_hypertable('merge_sparse_order', 'time', chunk_time_interval => INTERVAL '1 day');
1068+
create_hypertable
1069+
----------------------------------
1070+
(11,public,merge_sparse_order,t)
1071+
1072+
INSERT INTO merge_sparse_order
1073+
SELECT t, (i % 10) + 1, random() * 100, (i * 7) % 13
1074+
FROM generate_series('2024-01-01 2:00'::timestamptz, '2024-01-01 23:59', '1 minute') t,
1075+
generate_series(1, 5) i;
1076+
INSERT INTO merge_sparse_order
1077+
SELECT t, (i % 10) + 1, random() * 100, (i * 7) % 13
1078+
FROM generate_series('2024-01-02 2:00'::timestamptz, '2024-01-02 23:59', '1 minute') t,
1079+
generate_series(1, 5) i;
1080+
SELECT format('%I.%I', schema_name, table_name) AS mso_c1
1081+
FROM _timescaledb_catalog.chunk
1082+
WHERE hypertable_id = (SELECT id FROM _timescaledb_catalog.hypertable
1083+
WHERE table_name = 'merge_sparse_order')
1084+
ORDER BY id LIMIT 1 \gset
1085+
SELECT format('%I.%I', schema_name, table_name) AS mso_c2
1086+
FROM _timescaledb_catalog.chunk
1087+
WHERE hypertable_id = (SELECT id FROM _timescaledb_catalog.hypertable
1088+
WHERE table_name = 'merge_sparse_order')
1089+
ORDER BY id OFFSET 1 LIMIT 1 \gset
1090+
ALTER TABLE merge_sparse_order SET (
1091+
timescaledb.compress,
1092+
timescaledb.compress_orderby = 'time',
1093+
timescaledb.compress_index = 'bloom("device"), minmax("temp"), bloom("device","val")'
1094+
);
1095+
SELECT compress_chunk(:'mso_c1');
1096+
compress_chunk
1097+
------------------------------------------
1098+
_timescaledb_internal._hyper_11_32_chunk
1099+
1100+
ALTER TABLE merge_sparse_order SET (
1101+
timescaledb.compress,
1102+
timescaledb.compress_orderby = 'time',
1103+
timescaledb.compress_index = 'minmax("temp"), bloom("device")'
1104+
);
1105+
NOTICE: updated compression settings will only apply to future compressions
1106+
SELECT compress_chunk(:'mso_c2');
1107+
compress_chunk
1108+
------------------------------------------
1109+
_timescaledb_internal._hyper_11_33_chunk
1110+
1111+
-- have different sparse indexes
1112+
SELECT relid, index FROM _timescaledb_catalog.compression_settings ORDER BY relid::text;
1113+
relid | index
1114+
------------------------------------------+------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
1115+
_timescaledb_internal._hyper_1_1_chunk | [{"type": "minmax", "column": "time", "source": "orderby"}]
1116+
_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"}]
1117+
_timescaledb_internal._hyper_11_33_chunk | [{"type": "minmax", "column": "temp", "source": "config"}, {"type": "bloom", "column": "device", "source": "config"}, {"type": "minmax", "column": "time", "source": "orderby"}]
1118+
merge_sparse_order | [{"type": "minmax", "column": "temp", "source": "config"}, {"type": "bloom", "column": "device", "source": "config"}, {"type": "minmax", "column": "time", "source": "orderby"}]
1119+
mergeme | [{"type": "minmax", "column": "time", "source": "orderby"}]
1120+
1121+
-- Merge should not succeed
1122+
\set ON_ERROR_STOP 0
1123+
CALL merge_chunks(:'mso_c1'::regclass, :'mso_c2'::regclass);
1124+
ERROR: cannot merge compressed chunks with different compression settings
1125+
\set ON_ERROR_STOP 1
1126+
ALTER TABLE merge_sparse_order SET (
1127+
timescaledb.compress,
1128+
timescaledb.compress_orderby = 'time',
1129+
timescaledb.compress_index = 'bloom("val","device"), minmax("temp"), bloom("device")'
1130+
);
1131+
NOTICE: updated compression settings will only apply to future compressions
1132+
SELECT compress_chunk(decompress_chunk(:'mso_c2'));
1133+
compress_chunk
1134+
------------------------------------------
1135+
_timescaledb_internal._hyper_11_33_chunk
1136+
1137+
-- have different JSONB but same logical sparse indexes
1138+
SELECT relid, index FROM _timescaledb_catalog.compression_settings ORDER BY relid::text;
1139+
relid | index
1140+
------------------------------------------+------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
1141+
_timescaledb_internal._hyper_1_1_chunk | [{"type": "minmax", "column": "time", "source": "orderby"}]
1142+
_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"}]
1143+
_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"}]
1144+
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"}]
1145+
mergeme | [{"type": "minmax", "column": "time", "source": "orderby"}]
1146+
1147+
-- Merge should succeed
1148+
CALL merge_chunks(:'mso_c1'::regclass, :'mso_c2'::regclass);
1149+
SELECT relid, index FROM _timescaledb_catalog.compression_settings ORDER BY relid::text;
1150+
relid | index
1151+
------------------------------------------+------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
1152+
_timescaledb_internal._hyper_1_1_chunk | [{"type": "minmax", "column": "time", "source": "orderby"}]
1153+
_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"}]
1154+
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"}]
1155+
mergeme | [{"type": "minmax", "column": "time", "source": "orderby"}]
1156+
1157+
DROP TABLE merge_sparse_order;

0 commit comments

Comments
 (0)