Re: [PATCH] Add tests for Bitmapset
Greg Burd <greg@burd.me>
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:03:21Z
Lists: pgsql-hackers
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 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 →
-
Minor fixes for test_bitmapset.c
- 5142f0093e64 19 (unreleased) landed
-
test_bitmapset: Improve random function
- 9d46b86529e8 19 (unreleased) landed
-
Add mem_exceeded_count column to pg_stat_replication_slots.
- d3b6183dd988 19 (unreleased) landed
-
Tidy-up unneeded NULL parameter checks from SQL function
- a69b55cd4727 19 (unreleased) landed
-
Remove check for NULL in STRICT function
- 381f5cffae00 19 (unreleased) landed
-
Fixes for comments in test_bitmapset
- a1b064e4b226 19 (unreleased) landed
-
Minor fixups of test_bitmapset.c
- 3a6615806859 19 (unreleased) landed
-
test_bitmapset: Simplify code of the module
- 9952f6c05a40 19 (unreleased) landed
-
test_bitmapset: Expand more the test coverage
- 5668fff3c512 19 (unreleased) landed
-
Fix compiler warnings in test_bitmapset
- f83fe65f3fc1 19 (unreleased) landed
-
Add a test module for Bitmapset
- 00c3d87a5cab 19 (unreleased) landed