Content-Type: text/plain;
	charset=us-ascii
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3776.700.51.11.2\))
Subject: Re: [PATCH] Add tests for Bitmapset
From: Daniel Gustafsson <daniel@yesql.se>
In-Reply-To: 
 <CAApHDvpYkS+=KPcCvYOF4Za9=a_D94Gd9x1oCzuz97He1_T0Pw@mail.gmail.com>
Date: Thu, 2 Oct 2025 14:33:09 +0200
Cc: Michael Paquier <michael@paquier.xyz>,
 Greg Burd <greg@burd.me>,
 PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Content-Transfer-Encoding: quoted-printable
Message-Id: <BC483901-9587-4076-B20F-9A414C66AB78@yesql.se>
References: 
 <CAApHDvq-4994jpDnspQ+bgSbzo0+=Z4YQbj+YfP7wi97B0e1qg@mail.gmail.com>
 <76DBEF5D-EA47-4C30-8D28-97B3B88E2E2D@greg.burd.me>
 <CAApHDvrvixMmscGEfEyYz0=qbwzM7Y0Up-Uh_AKVmfn8K2sn_A@mail.gmail.com>
 <aNxpxE6a9G3sbfrm@paquier.xyz>
 <CAApHDvoC-W3s2QJdBY6B32BXy2br9iGsNJ3v4uLFOosBgmY2YA@mail.gmail.com>
 <aNyiu0fKRqihdwpV@paquier.xyz>
 <CAApHDvrEPw6DAQdZg7u1YknVvN1Gy38kUURQm8bOHSKU2cw+4w@mail.gmail.com>
 <aNy4xV4Fa1m9lyXX@paquier.xyz>
 <978D21E8-9D3B-40EA-A4B1-F87BABE7868C@yesql.se>
 <CAApHDvpYkS+=KPcCvYOF4Za9=a_D94Gd9x1oCzuz97He1_T0Pw@mail.gmail.com>
To: David Rowley <dgrowleyml@gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/BC483901-9587-4076-B20F-9A414C66AB78%40yesql.se>
Precedence: bulk

> On 2 Oct 2025, at 14:11, David Rowley <dgrowleyml@gmail.com> wrote:
>=20
> On Thu, 2 Oct 2025 at 23:00, Daniel Gustafsson <daniel@yesql.se> =
wrote:
>> Doing post-commit review I didn't see any sharp edges, but found one =
more of
>> these so will push this shortly:
>=20
> 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 =3D PG_GETARG_INT32(0);
        bms =3D bms_make_singleton(member);

I'll include all of these three tiny improvements in a commit later =
today.

--
Daniel Gustafsson