public inbox for [email protected]
help / color / mirror / Atom feedFrom: Ranier Vilela <[email protected]>
To: Greg Burd <[email protected]>
Cc: Daniel Gustafsson <[email protected]>
Cc: David Rowley <[email protected]>
Cc: Michael Paquier <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: [PATCH] Add tests for Bitmapset
Date: Wed, 8 Oct 2025 09:10:16 -0300
Message-ID: <CAEudQAq_zOSA2NUQSWePTGV_=90Uw0WcXxGOWnN-vwF046OOqA@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
<[email protected]>
Hi.
Em sex., 3 de out. de 2025 às 09:13, Greg Burd <[email protected]> escreveu:
>
> On Oct 3 2025, at 4:25 am, Daniel Gustafsson <[email protected]> wrote:
>
> >> On 3 Oct 2025, at 01:36, David Rowley <[email protected]> wrote:
> >>
> >> On Fri, 3 Oct 2025 at 01:33, Daniel Gustafsson <[email protected]> wrote:
> >>> Another nitpick would be to remove the test for NULL in
> test_bms_make_singleton
> >>> since that is a STRICT function, making the test for NULL
> >>> superfluous code:
> >>
> >> I see test_random_operations() is also strict. Is it worth getting rid
> >> of the SQL NULL checks on the inputs there too? Aka, the attached.
> >
> > Indeed, but reading the code I wonder if STRICT was a mistake and the
> intention
> > was to allow NULL input?
>
> Yes, it was an oversight after I re-worked the random function.
>
> > That being said, the function is never called with
> > NULL so that's mostly academic thinking. +1 for removing the NULL
> > checks and simplifying the code.
>
> I agree, and thank you both for the attention to detail and interest in
> this test suite.
>
With the patch attached, there are regression.
Is it intentional not to check the return of the function bms_is_member?
diff --strip-trailing-cr -U3
C:/dll/postgres_dev/postgres_commit/src/test/modules/test_bitmapset/expected/test_bitmapset.out
C:/dll/postgres_dev/postgres_commit/build/testrun/test_bitmapset/regress/results/test_bitmapset.out
---
C:/dll/postgres_dev/postgres_commit/src/test/modules/test_bitmapset/expected/test_bitmapset.out
2025-10-02 21:17:53.169515700 -0300
+++
C:/dll/postgres_dev/postgres_commit/build/testrun/test_bitmapset/regress/results/test_bitmapset.out
2025-10-07 21:24:00.534142300 -0300
@@ -1570,9 +1570,5 @@
-- random operations
SELECT test_random_operations(-1, 10000, 81920, 0) > 0 AS result;
- result
---------
- t
-(1 row)
-
+ERROR: union missing member 63904
DROP EXTENSION test_bitmapset;
best regards,
Ranier Vilela
diff --git a/src/test/modules/test_bitmapset/test_bitmapset.c b/src/test/modules/test_bitmapset/test_bitmapset.c
index 8bc9b1f48e..9e757732c5 100644
--- a/src/test/modules/test_bitmapset/test_bitmapset.c
+++ b/src/test/modules/test_bitmapset/test_bitmapset.c
@@ -700,7 +700,8 @@ test_random_operations(PG_FUNCTION_ARGS)
case 2: /* test membership */
if (bms != NULL)
{
- bms_is_member(member, bms);
+ if (!bms_is_member(member, bms))
+ elog(ERROR, "union missing member %d", member);
}
break;
}
Attachments:
[text/plain] check-function-return-bms_is_member.txt (538B, 3-check-function-return-bms_is_member.txt)
download | inline diff:
diff --git a/src/test/modules/test_bitmapset/test_bitmapset.c b/src/test/modules/test_bitmapset/test_bitmapset.c
index 8bc9b1f48e..9e757732c5 100644
--- a/src/test/modules/test_bitmapset/test_bitmapset.c
+++ b/src/test/modules/test_bitmapset/test_bitmapset.c
@@ -700,7 +700,8 @@ test_random_operations(PG_FUNCTION_ARGS)
case 2: /* test membership */
if (bms != NULL)
{
- bms_is_member(member, bms);
+ if (!bms_is_member(member, bms))
+ elog(ERROR, "union missing member %d", member);
}
break;
}
view thread (81+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: [PATCH] Add tests for Bitmapset
In-Reply-To: <CAEudQAq_zOSA2NUQSWePTGV_=90Uw0WcXxGOWnN-vwF046OOqA@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox