Horton
Menu

Re: [PATCH] Add tests for Bitmapset

Greg Burd <greg@burd.me>

View thread
From: Greg Burd <greg@burd.me>
To: Robert Haas <robertmhaas@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, Nathan Bossart <nathandbossart@gmail.com>, Masahiko Sawada <sawada.mshk@gmail.com>, Michael Paquier <michael@paquier.xyz>
Date: 2025-09-15T19:56:30Z
Lists: pgsql-hackers

Attachments

On Sep 15 2025, at 3:03 pm, Greg Burd <greg@burd.me> wrote:


> On Sep 15 2025, at 2:54 pm, Robert Haas <robertmhaas@gmail.com> wrote:
>  
>> On Mon, Sep 15, 2025 at 2:00 PM Greg Burd <greg@burd.me> wrote:
>>> For reference radixtree has:
>>>   
>>> coverage:      HEAD
>>>   lines......: 98.3
>>>   functions..: 97.2
>>>   branches...: 89.4
>>   
>> + /* Test negative member in bms_make_singleton */
>> + error_caught = false;
>> + PG_TRY();
>> + {
>> + bms_make_singleton(-1);
>> + }
>> + PG_CATCH();
>> + {
>> + error_caught = true;
>> + FlushErrorState();
>> + }
>> + PG_END_TRY();
>> + EXPECT_TRUE(error_caught);
>>   
>> This is an anti-pattern for PostgreSQL code. You can't just flush an
>> error without aborting a transaction or subtransaction to recover.
>> Even if it could be shown that this were harmless here, I think it's a
>> terrible idea to have code like this in the tree, as it encourages
>> people to do exactly the wrong thing.
>  
> Fair enough, I'll rework it.
>   
>> But backing up a step, this also doesn't really seem like the right
>> way to test the error conditions. It deliberately throws away the
>> error message. All this verifies is that you caught an error. If you
>> let the error escape to the client you could have the expected output
>> test that you got the expected message.
>>   
>> I think it would be a better idea to structure this as a set of
>> SQL-callable functions and move a bunch of the logic into SQL.
>  
> I'll give that approach a try, thanks for the suggestion.
>  
> -greg

Robert,

Reworked as indicated, thanks for pointing out the anti-pattern I'd
missed.

best.

-greg

>> --   
>> Robert Haas
>> EDB: http://www.enterprisedb.com

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