On Thu, Oct 09, 2025 at 02:57:41PM +1300, David Rowley wrote: > I don't think there's any need to maintain the order of that members > array, so couldn't you just do this?: > > bms = bms_del_member(bms, member); > members[pos] = members[--num_members];
Yep, I was just playing with all that and moving the last element to the member we know is gone would be cheaper. I have also added more comments to document everything in a more precise way, while going through. I also do not see a point in preventing inserts in the second set at the beginning of the function. This forces bms_union() to do more operations with overlapping sets. What do you think about the attached? -- Michael
From 03c6ee49b133e9f77dbd07e1df0a4e0aece511d7 Mon Sep 17 00:00:00 2001 From: Michael Paquier <[email protected]> Date: Thu, 9 Oct 2025 11:11:51 +0900 Subject: [PATCH v2] test_bitmapset: Improve random function --- .../modules/test_bitmapset/test_bitmapset.c | 48 ++++++++++++++----- 1 file changed, 36 insertions(+), 12 deletions(-) diff --git a/src/test/modules/test_bitmapset/test_bitmapset.c b/src/test/modules/test_bitmapset/test_bitmapset.c index 8bc9b1f48e9a..ce2034b43290 100644 --- a/src/test/modules/test_bitmapset/test_bitmapset.c +++ b/src/test/modules/test_bitmapset/test_bitmapset.c @@ -618,23 +618,23 @@ test_random_operations(PG_FUNCTION_ARGS) pg_prng_seed(&state, seed); members = palloc(sizeof(int) * num_ops); - /* Phase 1: Random insertions */ + /* Phase 1: Random insertions in first set */ for (int i = 0; i < num_ops / 2; i++) { member = pg_prng_uint32(&state) % max_range + min_value; if (!bms_is_member(member, bms1)) - { members[num_members++] = member; - bms1 = bms_add_member(bms1, member); - } + bms1 = bms_add_member(bms1, member); } - /* Phase 2: Random set operations */ + /* Phase 2: Random insertions in second set */ for (int i = 0; i < num_ops / 4; i++) { member = pg_prng_uint32(&state) % max_range + min_value; + if (!bms_is_member(member, bms2)) + members[num_members++] = member; bms2 = bms_add_member(bms2, member); } @@ -642,7 +642,7 @@ test_random_operations(PG_FUNCTION_ARGS) result = bms_union(bms1, bms2); EXPECT_NOT_NULL(result); - /* Verify union contains all members from first set */ + /* Verify union contains all members from first and second sets */ for (int i = 0; i < num_members; i++) { if (!bms_is_member(members[i], result)) @@ -650,7 +650,10 @@ test_random_operations(PG_FUNCTION_ARGS) } bms_free(result); - /* Test intersection */ + /* + * Test intersection, checking that all the members in the result are from + * both the first and second sets. + */ result = bms_intersect(bms1, bms2); if (result != NULL) { @@ -683,24 +686,44 @@ test_random_operations(PG_FUNCTION_ARGS) bms_free(bms1); bms_free(bms2); - for (int i = 0; i < num_ops; i++) + /* + * Phase 4: mix of operations on a single set, cross-checking a bitmap + * with a secondary state, "members". There can be up to "num_ops" + * members added, very unlikely still possible if all the operations hit + * the "0" case. + */ + num_members = 0; + members = palloc(sizeof(int) * num_ops); + + for (int op = 0; op < num_ops; op++) { - member = pg_prng_uint32(&state) % max_range + min_value; switch (pg_prng_uint32(&state) % 3) { case 0: /* add */ + member = pg_prng_uint32(&state) % max_range + min_value; + if (!bms_is_member(member, bms)) + members[num_members++] = member; bms = bms_add_member(bms, member); break; case 1: /* delete */ - if (bms != NULL) + if (num_members > 0) { + int pos = pg_prng_uint32(&state) % num_members; + + member = members[pos]; + if (!bms_is_member(member, bms)) + elog(ERROR, "expected %d to be a valid member", member); + bms = bms_del_member(bms, member); + members[pos] = members[--num_members]; } break; case 2: /* test membership */ - if (bms != NULL) + /* Verify that bitmap contains all members */ + for (int i = 0; i < num_members; i++) { - bms_is_member(member, bms); + if (!bms_is_member(members[i], bms)) + elog(ERROR, "missing member %d", members[i]); } break; } @@ -708,6 +731,7 @@ test_random_operations(PG_FUNCTION_ARGS) } bms_free(bms); + pfree(members); PG_RETURN_INT32(total_ops); } -- 2.51.0
signature.asc
Description: PGP signature
