From 0fa1882708b7048fa791cdab93f6fb2be0c8f846 Mon Sep 17 00:00:00 2001 From: David Rowley Date: Sat, 22 Aug 2026 14:14:59 +1200 Subject: [PATCH v1] Fix incorrect nmembers calculation in test_bitmapset This calculation was failing to correctly account for the min_value when calculating how many random members to add to the Bitmapset. What the code was meant to do was add a random number of members between the minimum and maximum values. The code failed to account for the minimum value being non-zero, which could result in too many members being added to the set, which could reduce the effectiveness of the test. In reality, the only caller to this function did pass 0 as the min_value, so there are no actual bugs being fixed here. In passing, swap the order of the min_value and max_value parameters. Having the minimum value appear before the maximum value makes more sense. Also align the names of both parameters, and adjust the 'offset' calculation to widen the offset range by 1 so that the possible range for that random value is -max_value to max_value. Author: David Rowley --- .../expected/test_bitmapset.out | 2 +- .../test_bitmapset/sql/test_bitmapset.sql | 2 +- .../modules/test_bitmapset/test_bitmapset.c | 53 ++++++++++++------- 3 files changed, 37 insertions(+), 20 deletions(-) diff --git a/src/test/modules/test_bitmapset/expected/test_bitmapset.out b/src/test/modules/test_bitmapset/expected/test_bitmapset.out index 18ccf402742..106898d09d5 100644 --- a/src/test/modules/test_bitmapset/expected/test_bitmapset.out +++ b/src/test/modules/test_bitmapset/expected/test_bitmapset.out @@ -1650,7 +1650,7 @@ SELECT test_random_operations(NULL, 10000, 81920, 0) > 0 AS result; (1 row) -- perform some random tests on bms_offset_members() -SELECT test_random_offset_operations(NULL, 1000, 1024, 0) AS result; +SELECT test_random_offset_operations(NULL, 1000, 0, 1024) AS result; result -------- 1000 diff --git a/src/test/modules/test_bitmapset/sql/test_bitmapset.sql b/src/test/modules/test_bitmapset/sql/test_bitmapset.sql index fd30b21f694..67ad57cf956 100644 --- a/src/test/modules/test_bitmapset/sql/test_bitmapset.sql +++ b/src/test/modules/test_bitmapset/sql/test_bitmapset.sql @@ -424,6 +424,6 @@ SELECT test_bms_nonempty_difference('(b 1 2)', '(b 50 100)') AS result; SELECT test_random_operations(NULL, 10000, 81920, 0) > 0 AS result; -- perform some random tests on bms_offset_members() -SELECT test_random_offset_operations(NULL, 1000, 1024, 0) AS result; +SELECT test_random_offset_operations(NULL, 1000, 0, 1024) AS result; DROP EXTENSION test_bitmapset; diff --git a/src/test/modules/test_bitmapset/test_bitmapset.c b/src/test/modules/test_bitmapset/test_bitmapset.c index 0f366aadea0..f7de9e7757e 100644 --- a/src/test/modules/test_bitmapset/test_bitmapset.c +++ b/src/test/modules/test_bitmapset/test_bitmapset.c @@ -790,8 +790,8 @@ test_random_operations(PG_FUNCTION_ARGS) * Arguments: * arg1: optional random seed. NULL means use a random seed. * arg2: the number of operations to perform. - * arg3: the maximum bitmapset member number to use in the random set. - * arg4: the minimum bitmapset member number to use in the random set. + * arg3: the minimum bitmapset member number to use in the random set. + * arg4: the maximum bitmapset member number to use in the random set. */ Datum test_random_offset_operations(PG_FUNCTION_ARGS) @@ -799,27 +799,32 @@ test_random_offset_operations(PG_FUNCTION_ARGS) pg_prng_state state; int64 seed; int num_ops; - int max_range; int min_value; + int max_value; int member; + uint32 range; if (PG_ARGISNULL(0)) seed = GetCurrentTimestamp(); else seed = PG_GETARG_INT64(0); - num_ops = PG_GETARG_INT32(1); - max_range = PG_GETARG_INT32(2); - min_value = PG_GETARG_INT32(3); - - if (PG_ARGISNULL(1) || num_ops <= 0) + if (PG_ARGISNULL(1) || PG_GETARG_INT32(1) <= 0) elog(ERROR, "invalid number of operations"); - if (PG_ARGISNULL(2) || max_range <= 0) - elog(ERROR, "invalid maximum range"); - if (PG_ARGISNULL(3) || min_value < 0) + if (PG_ARGISNULL(2) || PG_GETARG_INT32(2) < 0) elog(ERROR, "invalid minimum value"); + if (PG_ARGISNULL(3) || PG_GETARG_INT32(3) < 0) + elog(ERROR, "invalid maximum value"); + + num_ops = PG_GETARG_INT32(1); + min_value = PG_GETARG_INT32(2); + max_value = PG_GETARG_INT32(3); + + if (max_value < min_value) + elog(ERROR, "maximum value must be greater than or equal to minimum value"); pg_prng_seed(&state, (uint64) seed); + range = (uint32) max_value - (uint32) min_value + 1; for (int op = 0; op < num_ops; op++) { @@ -827,17 +832,29 @@ test_random_offset_operations(PG_FUNCTION_ARGS) Bitmapset *offset_bms1; Bitmapset *offset_bms2 = NULL; int offset; - int nmembers; + uint32 nmembers; CHECK_FOR_INTERRUPTS(); - /* Figure out a random offset and how many members to add */ - offset = (pg_prng_uint32(&state) % max_range) - (pg_prng_uint32(&state) % max_range); - nmembers = pg_prng_uint32(&state) % max_range + min_value; - - for (int i = 0; i < nmembers; i++) + /* + * Choose a random offset for passing to bms_offset_members(). We + * want a number between -max_value and max_value so we test both left + * and right shifting and also test cases that push members, + * occasionally all of them, off the bottom of the set. + */ + offset = (int) (pg_prng_uint32(&state) % ((uint32) max_value + 1)); + offset -= (int) (pg_prng_uint32(&state) % ((uint32) max_value + 1)); + + /* decide how many members to add */ + nmembers = pg_prng_uint32(&state) % range; + + /* + * Add a random number of members with values between the minimum and + * maximum values. + */ + for (uint32 i = 0; i < nmembers; i++) { - member = pg_prng_uint32(&state) % max_range + min_value; + member = min_value + (pg_prng_uint32(&state) % range); random_bms = bms_add_member(random_bms, member); } -- 2.53.0