Feedback-ID: i0fe9450f:Fastmail
Date: Mon, 20 Apr 2026 06:38:43 +0900
From: Michael Paquier <michael@paquier.xyz>
To: David Rowley <dgrowleyml@gmail.com>
Cc: Greg Burd <greg@burd.me>, Ranier Vilela <ranier.vf@gmail.com>,
	Daniel Gustafsson <daniel@yesql.se>,
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: [PATCH] Add tests for Bitmapset
Message-ID: <aeVLY650jkngPNE2@paquier.xyz>
References: 
 <CAEudQAq_zOSA2NUQSWePTGV_=90Uw0WcXxGOWnN-vwF046OOqA@mail.gmail.com>
 <2D90FFB0-C80A-4189-A5BF-C37F05E271D7@greg.burd.me>
 <CAApHDvqXdBWMwGuzJBSbq4eiNLDaR738-upd8NJpmhUq3a+XLQ@mail.gmail.com>
 <aOcaIr8QKMvbIlnA@paquier.xyz>
 <CAApHDvpn3bshX+_2t0NbJ263r1etcBdmkROjuJw8DvyZkMcXqg@mail.gmail.com>
 <aOcwU_0-U3zbNDki@paquier.xyz>
 <CAApHDvp=pEkO6LKfXKDjRNeQ1LkV7A5fsTn29ahUBXxmjyWPHg@mail.gmail.com>
 <aOg3X3BXjiXljHcR@paquier.xyz>
 <CAApHDvrDW9W72vAr7h7XeCu7+Qz-_Vff02Q+RPPuVeM0Qf0MCw@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="sRiIgLg3aXF1NC5O"
Content-Disposition: inline
In-Reply-To: 
 <CAApHDvrDW9W72vAr7h7XeCu7+Qz-_Vff02Q+RPPuVeM0Qf0MCw@mail.gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/aeVLY650jkngPNE2%40paquier.xyz>
Precedence: bulk


--sRiIgLg3aXF1NC5O
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Sat, Apr 18, 2026 at 09:06:02PM +1200, David Rowley wrote:
> On Fri, 10 Oct 2025 at 11:30, Michael Paquier <michael@paquier.xyz> wrote:
> > Thanks for double-checking.  Applied after running an indent.
>=20
> I was working on test_bitmapset.c to add some tests for a new
> bitmapset function. I noticed a few weird things.
>=20
> 1. test_random_operations() is coded to use GetCurrentTimestamp() as a
> seed when the given seed is <=3D 0. Of course, it'll be a while before
> the return value of that wraps beyond 2^63 (292250 years), but I still
> can't help but think that NULL is a better value to use to have the
> seed auto-generate.

Fine by me.

> 2. Doing #1 means the function can't be STRICT. I do think it's wrong
> that the function is marked as strict. That's normally reserved for
> functions that we needn't call because NULL input(s) yield a NULL
> output. That's not the case for this function.

Using the existing HEAD approach where STRICT avoids these extra NULL
checks, or adding explicit NULL checks without STRICT does not strike
me as a big difference in this context.

> 3. There's no CHECK_FOR_INTERRUPTS() in test_random_operations(). If
> someone uses a large num_ops, there's no way to cancel the query.
> 4. If there happened to be some rare bug in bitmapset.c that
> test_random_operations() we might struggle to find it again, as we
> don't report which seed we used in the ERROR message.

These make sense.

> I felt it was worth fixing these now as the function I plan to add
> there does #1, #2, #3 and #4. If I add the new function for v20, the
> discrepancy seems questionable.

It is a test module, it would be a big issue if new pieces are
backpatched in this area.  In short I'm fine with these.  Thanks for
asking.
--
Michael

--sRiIgLg3aXF1NC5O
Content-Type: application/pgp-signature; name=signature.asc

-----BEGIN PGP SIGNATURE-----

iQIzBAEBCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmnlS2MACgkQnvQgOdby
QH2LbBAAlK6IPT+5GduPO11fLJ7L7bkaXbBXqrqAE3CnfIYjYA04x9Ux9mGQQ4dM
RSkS7ghilibtT6W5gJpT2mG/q+y5qKd11ojj7wr+J7oaSvm1piRadO0DN1BheB5M
EhMjc9Y3bPm3BC377lqqJkwOnVwz+Qyzl/c2hrtxLuqbEafP8K51HYQfU2Xr6dXT
n9hBJuG3smiFZN+rIe9zOM1dI15P10Q71O7ff1gUkjKk4Jv4NaTJyu7y8yRnrsjR
P006VfpWhFuOOc6KaE4oQ+HGRrpeRqLoN8glLX0FXf18TDX/Ez/H9nECs5cWC4on
gwTa76NaGPeGzNQZ9wPpV1vxsPOKdIGuSUFchdmmbieuMnDcsSfwiPAliN/W3QFb
3QsqaM89lbv9sFfVtXL/Og6qL87ft9jc9qJ5nggTQ0qBYmt4/xqjMJt9j6BcwNKu
pLSk03dhnCH981bQQxS3kWxNhPpf4vA6mAZ/17PZM7wub8hwwaKXUxt2WBUkjdr/
vmOMHXr+RewDTdlzWpNSFRs1XoqlVkfx2vCQIJ6eMhPiS4FbdwlcAkvXnjFM5k24
7LS/cyi030K0T10/h1Of91pxZNsc7hRTcB9QN6sKgnkm4aaIBM68FjDSnjmOZn+C
s24/QiAlM3+D8sLC4+ltyTwOtwQRez2EQwJwjcqdJj0ZQ3oiNO4=
=5Ohy
-----END PGP SIGNATURE-----

--sRiIgLg3aXF1NC5O--