Assert in test_bms_membership();

Started by Ilya Cherdakov12 days ago5 messageshackers
Beta feature

Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.

needs rebasesuccessCI history

You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:

docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t253711
psql -h localhost -U postgres

Built from patchset v1 (message #1), September 09, 2026 at 12:10 PM.

Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:

git clone --branch t253711_1 https://github.com/hackorum-dev/postgres.git

In a checkout you already have, add the fork once:

git remote add hackorum https://github.com/hackorum-dev/postgres.git

then, for this patchset and every later one:

git fetch hackorum t253711_1 && git checkout t253711_1

Patchset v1 (message #1) is on t253711_1

Jump to latest
#1Ilya Cherdakov
i.cherdakov.pg@gmail.com

Postgresql 19 added a new module, src/test/modules/test_bitmapset.
It contains the function test_bms_membership(), which is a wrapper
around bms_membership. Passing some string that are not valid Bitmapset
representation to this function will result to assert. For example

SELECT test_bms_membership('b 1 2)');
or
SELECT test_bms_membership('true');

[backtrace.txt]

|The assertion appears to occur becaus| in
#define TEXT_TO_BITMAPSET(str) (test_bitmapset.c:91) casts to Bitmapset*
without type checking, using stringToNode, which obviously can return
types other than T_Bitmapset (read.c:247). This pointer, with the wrong
type, is then passed to bms_membership, where it fails the assert
with type checking (bitmapset.c:905).

The fix simply adds a check whether the type is valid or not,
as done in the bms_is_valid_set()(bitmapset.c:87) function.

---

Best regards,
Ilya Cherdakov
Postgres Professional: https://postgrespro.com

Attachments:

t253711_1
backtrace.txttext/plain; charset=UTF-8; name=backtrace.txtDownload
0001-Validate-Bitmapset-input-in-test_bitmapset.patchtext/plain; charset=UTF-8; name=0001-Validate-Bitmapset-input-in-test_bitmapset.patchDownload+15-3
#2Andres Freund
andres@anarazel.de
In reply to: Ilya Cherdakov (#1)
Re: Assert in test_bms_membership();

Hi,

On 2026-09-08 18:52:58 +0700, Ilya Cherdakov wrote:

Postgresql 19 added a new module, src/test/modules/test_bitmapset.
It contains the function test_bms_membership(), which is a wrapper
around bms_membership. Passing some string that are not valid Bitmapset
representation to this function will result to assert. For example

SELECT test_bms_membership('b 1 2)');
or
SELECT test_bms_membership('true');

[backtrace.txt]

|The assertion appears to occur becaus| in
#define TEXT_TO_BITMAPSET(str) (test_bitmapset.c:91) casts to Bitmapset*
without type checking, using stringToNode, which obviously can return
types other than T_Bitmapset (read.c:247). This pointer, with the wrong
type, is then passed to bms_membership, where it fails the assert
with type checking (bitmapset.c:905).

The fix simply adds a check whether the type is valid or not,
as done in the bms_is_valid_set()(bitmapset.c:87) function.

Perhaps a stupid question, but: Who cares? This is a test module, intended to
write tests for bms_*. I feel like we have much better things to do than to
prevent somebody triggering asserts while write tests in an intentionally bad
way?

Greetings,

Andres Freund

#3Zsolt Parragi
zsolt.parragi@percona.com
In reply to: Andres Freund (#2)
Re: Assert in test_bms_membership();

Hello!

This seem to be 0002 from a patchset I submitted yesterday[1]/messages/by-id/CAN4CZFOTQXMZvuPxweO4Fm7+sjrPJUqU-JvYKETLBxuhqgbPjw@mail.gmail.com.

Perhaps a stupid question, but: Who cares?

My reasoning for this was that the other test module that has the same issue (test_extensible) states that it is an example for extension developers:

This module can be used as a template for developers willing to develop
their own extensions. "Writing a Custom Scan Provider" covers the
documentation for custom scans.

So I thought a small fix is a good improvement there, and then test_bms_membership was an easy follow up because it's the same thing.

[1]: /messages/by-id/CAN4CZFOTQXMZvuPxweO4Fm7+sjrPJUqU-JvYKETLBxuhqgbPjw@mail.gmail.com

#4Andres Freund
andres@anarazel.de
In reply to: Zsolt Parragi (#3)
Re: Assert in test_bms_membership();

Hi,

On 2026-09-08 14:32:31 -0500, Zsolt Parragi wrote:

This seem to be 0002 from a patchset I submitted yesterday[1].

Perhaps a stupid question, but: Who cares?

My reasoning for this was that the other test module that has the same
issue (test_extensible) states that it is an example for extension
developers:

This module can be used as a template for developers willing to develop
their own extensions. "Writing a Custom Scan Provider" covers the
documentation for custom scans.

So I thought a small fix is a good improvement there, and then
test_bms_membership was an easy follow up because it's the same thing.

We are drowning in patches, with actual (not AI) review and committer
bandwidth being the bottleneck. I think showing that we accept patches for
making test modules not crash when held in bogus ways is a good way to make
that even worse.

I think using these modules as templates for real extensions tends to be a not
great idea. I turns out test code, which tends to lean towards testing edge
cases etc, is rarely nice, easy to understand, code.

Greetings,

Andres Freund

#5Michael Paquier
michael@paquier.xyz
In reply to: Andres Freund (#4)
Re: Assert in test_bms_membership();

On Tue, Sep 08, 2026 at 04:09:27PM -0400, Andres Freund wrote:

We are drowning in patches, with actual (not AI) review and committer
bandwidth being the bottleneck. I think showing that we accept patches for
making test modules not crash when held in bogus ways is a good way to make
that even worse.

With or without AI, drowning in patches for the past 8 years 4 months
has just been a part of my life. If somebody takes the time to send a
patch, I'll look at it. And here, FWIW, I don't mind much.

I think using these modules as templates for real extensions tends to be a not
great idea. I turns out test code, which tends to lean towards testing edge
cases etc, is rarely nice, easy to understand, code.

Depends, I guess.

Note: the other thread has a patch, let's keep things there.
--
Michael