Feedback-ID: i675e48f3:Fastmail
Date: Mon, 15 Sep 2025 15:03:21 -0400
From: Greg Burd <greg@burd.me>
To: Robert Haas <robertmhaas@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, Nathan
 Bossart <nathandbossart@gmail.com>, Masahiko Sawada
 <sawada.mshk@gmail.com>, Michael Paquier <michael@paquier.xyz>
Message-ID: <0FE0B26D-B94A-4BA7-BCA4-338C020609D9@greg.burd.me>
In-Reply-To: 
 <CA+TgmoYZfigfGgKsT2xCg6d8nhS+UFq6zrO6R6Aq+Dqz-=a5UA@mail.gmail.com>
References: 
 <CA+TgmoYZfigfGgKsT2xCg6d8nhS+UFq6zrO6R6Aq+Dqz-=a5UA@mail.gmail.com>
Subject: Re: [PATCH] Add tests for Bitmapset
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable
Content-Disposition: inline
Archived-At: 
 <https://www.postgresql.org/message-id/0FE0B26D-B94A-4BA7-BCA4-338C020609D9%40greg.burd.me>
Precedence: bulk


On Sep 15 2025, at 2:54 pm, Robert Haas <robertmhaas=40gmail.com> wrote:

> On Mon, Sep 15, 2025 at 2:00=E2=80=AFPM Greg Burd <greg=40burd.me> wrot=
e:
>> =46or reference radixtree has:
>> =20
>> coverage:      HEAD
>>   lines......: 98.3
>>   functions..: 97.2
>>   branches...: 89.4
> =20
> + /* Test negative member in bms=5Fmake=5Fsingleton */
> + error=5Fcaught =3D false;
> + PG=5FTRY();
> + =7B
> + bms=5Fmake=5Fsingleton(-1);
> + =7D
> + PG=5FCATCH();
> + =7B
> + error=5Fcaught =3D true;
> + =46lushErrorState();
> + =7D
> + PG=5FEND=5FTRY();
> + EXPECT=5FTRUE(error=5Fcaught);
> =20
> This is an anti-pattern for PostgreSQL code. You can't just flush an
> error without aborting a transaction or subtransaction to recover.
> Even if it could be shown that this were harmless here, I think it's a
> terrible idea to have code like this in the tree, as it encourages
> people to do exactly the wrong thing.

=46air enough, I'll rework it.
 =20
> But backing up a step, this also doesn't really seem like the right
> way to test the error conditions. It deliberately throws away the
> error message. All this verifies is that you caught an error. If you
> let the error escape to the client you could have the expected output
> test that you got the expected message.
> =20
> I think it would be a better idea to structure this as a set of
> SQL-callable functions and move a bunch of the logic into SQL.

I'll give that approach a try, thanks for the suggestion.

-greg

> -- =20
> Robert Haas
> EDB: http://www.enterprisedb.com