Skip to content

Commit 80d6cb8

Browse files
Poroma-Banerjeetimescale-automation
Poroma-Banerjee
authored and
timescale-automation
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. (cherry picked from commit 607bbd5)
1 parent a41ad32 commit 80d6cb8

6 files changed

Lines changed: 427 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
@@ -141,6 +141,7 @@ TSDLLEXPORT bool ts_compression_settings_equal(const CompressionSettings *left,
141141
const CompressionSettings *right);
142142
TSDLLEXPORT bool ts_compression_settings_equal_with_defaults(const CompressionSettings *ht,
143143
const CompressionSettings *chunk);
144+
TSDLLEXPORT bool ts_sparse_index_equal(const Jsonb *left, const Jsonb *right);
144145

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

‎test/src/test_compression_settings.c‎

Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -337,10 +337,143 @@ 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+
/* Extra keys — not equal */
435+
{
436+
Jsonb *a =
437+
cstring_to_jsonb("[{\"type\": \"bloom\", \"column\": \"x\", \"source\": \"config\"}]");
438+
Jsonb *b = cstring_to_jsonb(
439+
"[{\"type\": \"bloom\", \"column\": \"x\", \"source\": \"config\",\"foo\":\"bar\"}]");
440+
TestAssertBoolEq(ts_sparse_index_equal(a, b), false);
441+
pfree(a);
442+
pfree(b);
443+
}
444+
445+
/* Different source — not equal */
446+
{
447+
Jsonb *a = cstring_to_jsonb(
448+
"[{\"type\": \"minmax\", \"column\": \"ts\", \"source\": \"config\"}]");
449+
Jsonb *b = cstring_to_jsonb(
450+
"[{\"type\": \"minmax\", \"column\": \"ts\", \"source\": \"orderby\"}]");
451+
TestAssertBoolEq(ts_sparse_index_equal(a, b), false);
452+
pfree(a);
453+
pfree(b);
454+
}
455+
456+
/* Three objects shuffled — equal */
457+
{
458+
Jsonb *a = cstring_to_jsonb(
459+
"[{\"type\": \"bloom\", \"column\": \"a\", \"source\": \"config\"}, "
460+
"{\"type\": \"bloom\", \"column\": \"b\", \"source\": \"config\"}, "
461+
"{\"type\": \"minmax\", \"column\": \"ts\", \"source\": \"orderby\"}]");
462+
Jsonb *b = cstring_to_jsonb(
463+
"[{\"type\": \"minmax\", \"column\": \"ts\", \"source\": \"orderby\"}, "
464+
"{\"type\": \"bloom\", \"column\": \"b\", \"source\": \"config\"}, "
465+
"{\"type\": \"bloom\", \"column\": \"a\", \"source\": \"config\"}]");
466+
TestAssertBoolEq(ts_sparse_index_equal(a, b), true);
467+
pfree(a);
468+
pfree(b);
469+
}
470+
}
471+
340472
TS_TEST_FN(ts_test_compression_settings)
341473
{
342474
test_alter_table_rename_column_effect_jsonb();
343475
test_alter_table_drop_column_effect_jsonb();
344476
test_convert_to_sparse_index_settings();
477+
test_sparse_index_equal();
345478
PG_RETURN_VOID();
346479
}

0 commit comments

Comments
 (0)