public inbox for [email protected]
help / color / mirror / Atom feedFrom: Michael Paquier <[email protected]>
To: David Rowley <[email protected]>
Cc: Greg Burd <[email protected]>
Cc: Ranier Vilela <[email protected]>
Cc: Daniel Gustafsson <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: [PATCH] Add tests for Bitmapset
Date: Thu, 9 Oct 2025 11:12:50 +0900
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAApHDvqXdBWMwGuzJBSbq4eiNLDaR738-upd8NJpmhUq3a+XLQ@mail.gmail.com>
References: <CAEudQAq_zOSA2NUQSWePTGV_=90Uw0WcXxGOWnN-vwF046OOqA@mail.gmail.com>
<[email protected]>
<CAApHDvqXdBWMwGuzJBSbq4eiNLDaR738-upd8NJpmhUq3a+XLQ@mail.gmail.com>
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
Attachments:
[text/x-diff] v2-0001-test_bitmapset-Improve-random-function.patch (3.6K, 2-v2-0001-test_bitmapset-Improve-random-function.patch)
download | inline diff:
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
[application/pgp-signature] signature.asc (833B, 3-signature.asc)
download
view thread (81+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: [PATCH] Add tests for Bitmapset
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox