public inbox for [email protected]
help / color / mirror / Atom feedFrom: Daniel Gustafsson <[email protected]>
To: David Rowley <[email protected]>
Cc: Michael Paquier <[email protected]>
Cc: Greg Burd <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: [PATCH] Add tests for Bitmapset
Date: Thu, 2 Oct 2025 14:33:09 +0200
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAApHDvpYkS+=KPcCvYOF4Za9=a_D94Gd9x1oCzuz97He1_T0Pw@mail.gmail.com>
References: <CAApHDvq-4994jpDnspQ+bgSbzo0+=Z4YQbj+YfP7wi97B0e1qg@mail.gmail.com>
<[email protected]>
<CAApHDvrvixMmscGEfEyYz0=qbwzM7Y0Up-Uh_AKVmfn8K2sn_A@mail.gmail.com>
<[email protected]>
<CAApHDvoC-W3s2QJdBY6B32BXy2br9iGsNJ3v4uLFOosBgmY2YA@mail.gmail.com>
<[email protected]>
<CAApHDvrEPw6DAQdZg7u1YknVvN1Gy38kUURQm8bOHSKU2cw+4w@mail.gmail.com>
<[email protected]>
<[email protected]>
<CAApHDvpYkS+=KPcCvYOF4Za9=a_D94Gd9x1oCzuz97He1_T0Pw@mail.gmail.com>
> On 2 Oct 2025, at 14:11, David Rowley <[email protected]> wrote:
>
> On Thu, 2 Oct 2025 at 23:00, Daniel Gustafsson <[email protected]> wrote:
>> Doing post-commit review I didn't see any sharp edges, but found one more of
>> these so will push this shortly:
>
> Any chance you could also delete the "/* memory cleanup seems more
> tricky than it's worth here */" line? That comment seemed relevant
> when we were actually doing bms_free() in the functions. Now that
> we're not, it's a bit out of place.
Sure I can take care of that while in there.
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:
diff --git a/src/test/modules/test_bitmapset/test_bitmapset.c b/src/test/modules/test_bitmapset/test_bitmapset.c
index 0d6c2e7aa1b..acaa93d2f11 100644
--- a/src/test/modules/test_bitmapset/test_bitmapset.c
+++ b/src/test/modules/test_bitmapset/test_bitmapset.c
@@ -201,9 +201,6 @@ test_bms_make_singleton(PG_FUNCTION_ARGS)
Bitmapset *bms;
int32 member;
- if (PG_ARGISNULL(0))
- PG_RETURN_NULL(); /* invalid input */
-
member = PG_GETARG_INT32(0);
bms = bms_make_singleton(member);
I'll include all of these three tiny improvements in a commit later today.
--
Daniel Gustafsson
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]
Subject: Re: [PATCH] Add tests for Bitmapset
In-Reply-To: <[email protected]>
* 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