Minor fixes for test_bitmapset.c

1. Make it so test_random_operations() can accept a NULL to have the
   function select a random seed.
2. Widen the seed parameter of test_random_operations() to bigint.
   Without that, it'll be impossible to run the function with a seed
   which was selected by GetCurrentTimestamp(), and if a randomly
   selected seed ever results in a failure, we'll likely want to run
   with the same seed to debug the issue.
3. Report the seed in the error messages in test_random_operations().
   If the buildfarm were ever to fail there, we'd certainly want to know
   what this was.
4. Add CHECK_FOR_INTERRUPTS() to test_random_operations().  Someone might
   run with a large num_ops and they'd have no way to cancel the query.
5. Minor cosmetic fixes; header order and whitespace issue.

To allow #1, the STRICT modifier had to be removed.  The additional
prechecks were added as I didn't see how else to handle someone passing
those parameters as NULL.

Author: David Rowley <dgrowleyml@gmail.com>
Reviewed-by: Greg Burd <greg@burd.me>
Discussion: https://postgr.es/m/CAApHDvrDW9W72vAr7h7XeCu7+Qz-_Vff02Q+RPPuVeM0Qf0MCw@mail.gmail.com

diff --git a/src/test/modules/test_bitmapset/expected/test_bitmapset.out b/src/test/modules/test_bitmapset/expected/test_bitmapset.out
index f75cb46b86994915f904b5d9248f117e338f5d8b..0b72b91cd1f845644cf07a5c1376160d80e25e2b 100644 (file)
--- a/src/test/modules/test_bitmapset/expected/test_bitmapset.out
+++ b/src/test/modules/test_bitmapset/expected/test_bitmapset.out
@@ -1569,7 +1569,7 @@ SELECT test_bms_nonempty_difference('(b 1 2)', '(b 50 100)') AS result;
 (1 row)
 
 -- random operations
-SELECT test_random_operations(-1, 10000, 81920, 0) > 0 AS result;
+SELECT test_random_operations(NULL, 10000, 81920, 0) > 0 AS result;
  result 
 --------
  t

diff --git a/src/test/modules/test_bitmapset/sql/test_bitmapset.sql b/src/test/modules/test_bitmapset/sql/test_bitmapset.sql
index e75ab8f620a7009ce715e882a7c14a64db8e8b74..c53232e0ada57fb6b8c7d634820e77802ef8bfec 100644 (file)
--- a/src/test/modules/test_bitmapset/sql/test_bitmapset.sql
+++ b/src/test/modules/test_bitmapset/sql/test_bitmapset.sql
@@ -401,6 +401,6 @@ SELECT test_bms_nonempty_difference('(b 100)', '(b 5)') AS result;
 SELECT test_bms_nonempty_difference('(b 1 2)', '(b 50 100)') AS result;
 
 -- random operations
-SELECT test_random_operations(-1, 10000, 81920, 0) > 0 AS result;
+SELECT test_random_operations(NULL, 10000, 81920, 0) > 0 AS result;
 
 DROP EXTENSION test_bitmapset;

diff --git a/src/test/modules/test_bitmapset/test_bitmapset--1.0.sql b/src/test/modules/test_bitmapset/test_bitmapset--1.0.sql
index 227ecb5aa3b5068ebd854e1584718e1f3b80ea3d..e7b263e51f54335c15b32d6661cbbe9a587df46f 100644 (file)
--- a/src/test/modules/test_bitmapset/test_bitmapset--1.0.sql
+++ b/src/test/modules/test_bitmapset/test_bitmapset--1.0.sql
@@ -133,8 +133,8 @@ RETURNS int
 AS 'MODULE_PATHNAME' LANGUAGE C;
 
 -- Test utility functions
-CREATE FUNCTION test_random_operations(integer, integer, integer, integer)
-RETURNS integer STRICT
+CREATE FUNCTION test_random_operations(bigint, integer, integer, integer)
+RETURNS integer
 AS 'MODULE_PATHNAME' LANGUAGE C;
 
 COMMENT ON EXTENSION test_bitmapset IS 'Test code for Bitmapset';

diff --git a/src/test/modules/test_bitmapset/test_bitmapset.c b/src/test/modules/test_bitmapset/test_bitmapset.c
index 272bed390a6b8e4601bb82c3ba833f236a39dd4b..3a185369651b930fd54833c3f8762c4220a4ca6a 100644 (file)
--- a/src/test/modules/test_bitmapset/test_bitmapset.c
+++ b/src/test/modules/test_bitmapset/test_bitmapset.c
@@ -1,7 +1,7 @@
 /*-------------------------------------------------------------------------
  *
  * test_bitmapset.c
- *      Test the Bitmapset data structure.
+ *       Test the Bitmapset data structure.
  *
  * This module tests the Bitmapset implementation in PostgreSQL, covering
  * all public API functions.
@@ -19,11 +19,12 @@
 #include <stddef.h>
 #include "catalog/pg_type.h"
 #include "common/pg_prng.h"
-#include "utils/array.h"
 #include "fmgr.h"
+#include "miscadmin.h"
 #include "nodes/bitmapset.h"
 #include "nodes/nodes.h"
 #include "nodes/pg_list.h"
+#include "utils/array.h"
 #include "utils/builtins.h"
 #include "utils/timestamp.h"
 
@@ -587,7 +588,7 @@ test_bitmap_match(PG_FUNCTION_ARGS)
  * "min_value" is the minimal value used for the members, that will stand
  * up to a range of "max_range".  "num_ops" defines the number of time each
  * operation is done.  "seed" is a random seed used to calculate the member
- * values.  When "seed" is <= 0, a random seed will be chosen automatically.
+ * values.  When "seed" is NULL, a random seed will be chosen automatically.
  *
  * The return value is the number of times all operations have been executed.
  */
@@ -608,13 +609,20 @@ test_random_operations(PG_FUNCTION_ARGS)
        int                     num_members = 0;
        int                     total_ops = 0;
 
-       if (PG_GETARG_INT32(0) > 0)
-               seed = PG_GETARG_INT32(0);
+       if (!PG_ARGISNULL(0))
+               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)
+               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)
+               elog(ERROR, "invalid minimum value");
+
        pg_prng_seed(&state, seed);
 
        /*
@@ -627,6 +635,8 @@ test_random_operations(PG_FUNCTION_ARGS)
        /* Phase 1: Random insertions in first set */
        for (int i = 0; i < num_ops / 2; i++)
        {
+               CHECK_FOR_INTERRUPTS();
+
                member = pg_prng_uint32(&state) % max_range + min_value;
 
                if (!bms_is_member(member, bms1))
@@ -637,6 +647,8 @@ test_random_operations(PG_FUNCTION_ARGS)
        /* Phase 2: Random insertions in second set */
        for (int i = 0; i < num_ops / 4; i++)
        {
+               CHECK_FOR_INTERRUPTS();
+
                member = pg_prng_uint32(&state) % max_range + min_value;
 
                if (!bms_is_member(member, bms2))
@@ -651,8 +663,11 @@ test_random_operations(PG_FUNCTION_ARGS)
        /* Verify union contains all members from first and second sets */
        for (int i = 0; i < num_members; i++)
        {
+               CHECK_FOR_INTERRUPTS();
+
                if (!bms_is_member(members[i], result))
-                       elog(ERROR, "union missing member %d", members[i]);
+                       elog(ERROR, "union missing member %d, seed " INT64_FORMAT,
+                                members[i], seed);
        }
        bms_free(result);
 
@@ -667,8 +682,11 @@ test_random_operations(PG_FUNCTION_ARGS)
 
                while ((member = bms_next_member(result, member)) >= 0)
                {
+                       CHECK_FOR_INTERRUPTS();
+
                        if (!bms_is_member(member, bms1) || !bms_is_member(member, bms2))
-                               elog(ERROR, "intersection contains invalid member %d", member);
+                               elog(ERROR, "intersection contains invalid member %d, seed " INT64_FORMAT,
+                                        member, seed);
                }
                bms_free(result);
        }
@@ -680,6 +698,8 @@ test_random_operations(PG_FUNCTION_ARGS)
                int                     lower = pg_prng_uint32(&state) % 100;
                int                     upper = lower + (pg_prng_uint32(&state) % 20);
 
+               CHECK_FOR_INTERRUPTS();
+
                result = bms_add_range(result, lower, upper);
        }
        if (result != NULL)
@@ -699,6 +719,8 @@ test_random_operations(PG_FUNCTION_ARGS)
 
        for (int op = 0; op < num_ops; op++)
        {
+               CHECK_FOR_INTERRUPTS();
+
                switch (pg_prng_uint32(&state) % 3)
                {
                        case 0:                         /* add */
@@ -714,7 +736,8 @@ test_random_operations(PG_FUNCTION_ARGS)
 
                                        member = members[pos];
                                        if (!bms_is_member(member, bms))
-                                               elog(ERROR, "expected %d to be a valid member", member);
+                                               elog(ERROR, "expected %d to be a valid member, seed " INT64_FORMAT,
+                                                        member, seed);
 
                                        bms = bms_del_member(bms, member);
 
@@ -730,7 +753,8 @@ test_random_operations(PG_FUNCTION_ARGS)
                                for (int i = 0; i < num_members; i++)
                                {
                                        if (!bms_is_member(members[i], bms))
-                                               elog(ERROR, "missing member %d", members[i]);
+                                               elog(ERROR, "missing member %d, seed " INT64_FORMAT,
+                                                        members[i], seed);
                                }
                                break;
                }