public inbox for [email protected]
help / color / mirror / Atom feedFrom: David Rowley <[email protected]>
To: Tom Lane <[email protected]>
Cc: Chao Li <[email protected]>
Cc: PostgreSQL Developers <[email protected]>
Subject: Re: Add bms_offset_members() function for bitshifting Bitmapsets
Date: Wed, 15 Apr 2026 14:21:36 +1200
Message-ID: <CAApHDvq4_WDfTaWX6nHU8TV+no+aD1zjLphza685Ek7=xgGzew@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <CAApHDvq=eEdw2Qp+aSzSOtTSe+h0fnVQ55CcTNqBkLDYiRZmxw@mail.gmail.com>
<[email protected]>
<CAApHDvoALVPgVSCuGdAfNUbxx_NacnAd0nuj8Hi=_-61AEqzaQ@mail.gmail.com>
<[email protected]>
On Wed, 15 Apr 2026 at 12:29, Tom Lane <[email protected]> wrote:
> I question the decision to make this change the set in-place.
> Wouldn't it be cheaper and less surprise-prone to always make
> a copy?
I'd not considered surprise-prone as an aspect. I understand we have
bms_join and bms_union, which do the same thing if you only care about
the value of the result and not what happens to the inputs. So I
didn't think I was introducing anything too surpising given we've got
various other Bitmapset functions that modify the input in-place. My
expectation there was that people are used to checking what the
behaviour of the bitmapset function is.
For the current use cases of the function in the patch, I agree that
it would likely be better for performance if the new function
allocated a new set. It was more a question of whether we want to
throw away performance for other cases which are fine with an in-place
update and have a positive offset. Those will never repalloc(). I
didn't really know the answer. I suspect with the current patch that
each of the existing cases will be faster than today's bms_next_member
loops, regardless. When I wrote the function, I was mainly thinking
of the new use-case that I was working on, which won't require any
palloc() or repalloc() with the current design. Since I was adding
that to a fairly common code path, I thought I had more of a chance of
not having to jump through too many hoops to ensure I don't cause any
performance regressions.
In short, I don't really know what's best. I'm certainly open to
changing it if someone comes up with a good reason to do it the other
way. Maybe catering for the majority of callers is a good enough
reason to change it.
David
view thread (15+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected]
Subject: Re: Add bms_offset_members() function for bitshifting Bitmapsets
In-Reply-To: <CAApHDvq4_WDfTaWX6nHU8TV+no+aD1zjLphza685Ek7=xgGzew@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox