Re: Refactor how we form HeapTuples for CatalogTuple(Insert|Update)

"Greg Burd" <greg(at)burd(dot)me> writes:
> On Fri, Jan 30, 2026, at 12:20 PM, Andres Freund wrote:
>> Sorry for being a downer - but: Is the gain here really worth the squeeze?

> You are certainly not wrong, there are a lot of changes in this patch set. To be honest, I'm not sure if it is "worth the squeeze" either.

> What this patch does: it removes the need to rediscover the set of indexed attributes that changed during catalog tuple updates.

> Impact of that change: as-yet-unmeasured performance gains due to not having to redo work while holding a lock on the heap page.

Keep in mind that a patch like this

* requires a ton of work to review
* breaks a lot of pending patches
* breaks a lot of extensions
* causes significant back-patching pain for the next five years

With that in mind, you'd have to show *massive* performance
improvements for it to have any chance of getting accepted.
Frankly I doubt it can possibly clear that bar.

You could lower the bar considerably if, instead of forcing a
system-wide API change, you maintained compatibility with the
existing API and introduced a new one alongside it, using the
new one only in selected code paths that seem particularly
performance-critical. That would remove the objection of
breaking extensions and pending patches, and probably greatly
reduce the other two costs as well, depending on how selective
you are about where to introduce the new API.

With that in mind, I really wonder why it's so critical to change
from replacement flags represented as array-of-bool to replacement
flags represented as a Bitmapset. Seems like those are fundamentally
equivalent. A Bitmapset is more compact, sure, but I fail to see
why we care about that in this context: the replacement flags are
typically function-local variables that won't live very long.
I'm also concerned that the palloc required to create a Bitmapset
will be a net drag on performance compared to a local array.

regards, tom lane