Feedback-ID: i675e48f3:Fastmail
MIME-Version: 1.0
Date: Sat, 13 Sep 2025 10:23:36 -0400
From: "Greg Burd" <greg@burd.me>
To: "Michael Paquier" <michael@paquier.xyz>,
 "Masahiko Sawada" <sawada.mshk@gmail.com>
Cc: "Nathan Bossart" <nathandbossart@gmail.com>,
 "PostgreSQL Hackers" <pgsql-hackers@lists.postgresql.org>
Message-Id: <20200C9C-8DA3-413F-AE9A-60D6761DD060@burd.me>
In-Reply-To: <aMN5Eu8RjREUpl_F@paquier.xyz>
References: <7BD1ABDB-B03A-464A-9BA9-A73B55AD8A1F@getmailspring.com>
 <aLpENihhKL4vOxeX@nathan> <02DB5E92-1E94-4617-AC11-836486F63BD5@burd.me>
 <aLsvPdSHJdRCEk7v@nathan> <9E1E0BA4-952C-412A-884A-6E700F26B0CA@burd.me>
 <CAD21AoDijQoXik+U8hf0QptPvNcpjCjfzO_Rp=eCLYSL1D6OOQ@mail.gmail.com>
 <0978d5ba-e015-4889-a8b7-7a2891664492@app.fastmail.com>
 <aMN5Eu8RjREUpl_F@paquier.xyz>
Subject: Re: [PATCH] Add tests for Bitmapset
Content-Type: multipart/alternative;
 boundary=a7dc806e6f014461b86faaf2c7ded1a1
Archived-At: 
 <https://www.postgresql.org/message-id/20200C9C-8DA3-413F-AE9A-60D6761DD060%40burd.me>
Precedence: bulk

--a7dc806e6f014461b86faaf2c7ded1a1
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable


> On Sep 11, 2025, at 9:36=E2=80=AFPM, Michael Paquier <michael@paquier.=
xyz> wrote:
>=20
> On Thu, Sep 11, 2025 at 06:56:07AM -0400, Greg Burd wrote:
>> Just for reference I started this not to increase coverage, which is =
a good
>> goal just not the one I had.  I was reviewing the API and considering=
 some
>> changes based on other work I've done.  Now that I see how deeply bak=
ed in
>> this code is I think that's unlikely.  Maybe something else distinct =
for
>> bitmaps over 64-bit space at some point will be useful.  I wrote this=
 code
>> just to capture the API in test form.
>=20
> How much does this measure in terms of numbers produced by
> coverage-html (see [1])?  The paths taken don't always matter as it
> can also be important to check combinations of code paths that are
> taken by other tests when checking after edge cases, but that would
> give an idea of gain vs extra runtime.  Not objecting to your patch,
> just being curious as I am not seeing any numbers posted on this
> thread.
>=20
> [1]: https://www.postgresql.org/docs/devel/regress-coverage.html
> --
> Michael


Sawada-san, Michael,

Thank you both for the push to measure.  Before the patch as it stands n=
ow the
coverage for src/backend/nodes/bitmapset.c is 63.5% and after it is 66.5=
%.  Not
an amazing difference, but something.  I guess I expected this to be hig=
her given
the degree to which this datatype is used.

I'll review the gaps in coverage and update the tests.  I'll look for a =
way to add
meaningful randomization.

-greg
--a7dc806e6f014461b86faaf2c7ded1a1
Content-Type: text/html; charset=utf-8
Content-Transfer-Encoding: quoted-printable

<!DOCTYPE html><html><head><title></title></head><body><div><br></div><b=
lockquote type=3D"cite"><div>On Sep 11, 2025, at 9:36=E2=80=AFPM, Michae=
l Paquier &lt;michael@paquier.xyz&gt; wrote:</div><div><br></div><div>On=
 Thu, Sep 11, 2025 at 06:56:07AM -0400, Greg Burd wrote:</div><blockquot=
e type=3D"cite"><div>Just for reference I started this not to increase c=
overage, which is a good</div><div>goal just not the one I had. &nbsp;I =
was reviewing the API and considering some</div><div>changes based on ot=
her work I've done. &nbsp;Now that I see how deeply baked in</div><div>t=
his code is I think that's unlikely. &nbsp;Maybe something else distinct=
 for</div><div>bitmaps over 64-bit space at some point will be useful. &=
nbsp;I wrote this code</div><div>just to capture the API in test form.</=
div></blockquote><div><br></div><div>How much does this measure in terms=
 of numbers produced by</div><div>coverage-html (see [1])? &nbsp;The pat=
hs taken don't always matter as it</div><div>can also be important to ch=
eck combinations of code paths that are</div><div>taken by other tests w=
hen checking after edge cases, but that would</div><div>give an idea of =
gain vs extra runtime. &nbsp;Not objecting to your patch,</div><div>just=
 being curious as I am not seeing any numbers posted on this</div><div>t=
hread.</div><div><br></div><div>[1]: https://www.postgresql.org/docs/dev=
el/regress-coverage.html</div><div>--</div><div>Michael</div></blockquot=
e><div><br></div><div><br></div><div>Sawada-san, Michael,</div><div><br>=
</div><div>Thank you both for the push to measure. &nbsp;Before the patc=
h as it stands now the</div><div>coverage for src/backend/nodes/bitmapse=
t.c is 63.5% and after it is 66.5%. &nbsp;Not</div><div>an amazing diffe=
rence, but something.&nbsp; I guess I expected this to be higher given</=
div><div>the degree to which this datatype is used.</div><div><br></div>=
<div>I'll review the gaps in coverage and update the tests.&nbsp; I'll l=
ook for a way to add</div><div>meaningful randomization.</div><div><br><=
/div><div>-greg</div></body></html>
--a7dc806e6f014461b86faaf2c7ded1a1--