Horton
Menu

Re: [PATCH] Add tests for Bitmapset

Greg Burd <greg@burd.me>

View thread
From: Greg Burd <greg@burd.me>
To: Michael Paquier <michael@paquier.xyz>, Masahiko Sawada <sawada.mshk@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, Nathan Bossart <nathandbossart@gmail.com>, Robert Haas <robertmhaas@gmail.com>
Date: 2025-09-17T11:59:15Z
Lists: pgsql-hackers 
On Sep 16 2025, at 8:35 pm, Michael Paquier <michael@paquier.xyz> wrote:

> On Tue, Sep 16, 2025 at 03:00:40PM -0700, Masahiko Sawada wrote:
>> Thank you for updating the patch. It seems cfbot caught a regression
>> test error[1] in a 32-bit build.
> 
> - bitmap_hash [1,3,5] |   49870778
> + bitmap_hash [1,3,5] | 1509752520
> 
> This one is able the hash value computation being not portable across
> architectures.  I would just change these to be non-NULL, I guess.
> --
> Michael

Thanks Michael, Sawada-san, yep hashing is arch dependent (go figure!),
but thankfully it is deterministic. Rather that test for non-NULL I've
come up with this approach:

-- Architecture-aware hash testing
WITH arch_info AS (
    SELECT
        CASE
            WHEN pg_column_size(1::bigint) = 8 THEN '64bit'
            ELSE '32bit'
        END as architecture
),
expected_values AS (
    SELECT
        architecture,
        CASE architecture
            WHEN '64bit' THEN 0
            WHEN '32bit' THEN 0
        END as hash_null,
        CASE architecture
            WHEN '64bit' THEN 49870778
            WHEN '32bit' THEN 1509752520
        END as hash_135,
        CASE architecture
            WHEN '64bit' THEN -303921606
            WHEN '32bit' THEN 0  -- TBD
        END as hash_246
    FROM arch_info
)
SELECT 'expected hash NULL' as test,
       test_bitmap_hash(NULL) = hash_null as result
FROM expected_values
UNION ALL
SELECT 'expected hash [1,3,5]' as test,
       test_bitmap_hash(test_bms_from_array(ARRAY[1,3,5])) = hash_135 as result
FROM expected_values
UNION ALL
SELECT 'expected hash [2,4,6]' as test,
       test_bitmap_hash(test_bms_from_array(ARRAY[2,4,6])) = hash_246 as result
FROM expected_values;

However I'm not sure what the value is for testing hash functions except
for coverage and I'm fairly certain that function is well exercised. 
That said, I think that will work.  I'll let cfbot tell me the 32bit
hash value on the next run.

thanks again for the helpful insights,

-greg

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Minor fixes for test_bitmapset.c

  2. test_bitmapset: Improve random function

  3. Add mem_exceeded_count column to pg_stat_replication_slots.

  4. Tidy-up unneeded NULL parameter checks from SQL function

  5. Remove check for NULL in STRICT function

  6. Fixes for comments in test_bitmapset

  7. Minor fixups of test_bitmapset.c

  8. test_bitmapset: Simplify code of the module

  9. test_bitmapset: Expand more the test coverage

  10. Fix compiler warnings in test_bitmapset

  11. Add a test module for Bitmapset