On Thu, Mar 12, 2026 at 10:43 AM Tomas Vondra <tomas@vondra.me> wrote:
> I took a quick look at 0001 and 0002 patches, and I think it's mostly
> ready to go.
Pushed both patches just now. Thanks for the review!
> I only have a couple very minor comments:
>
> 1) v12-0001-Use-simple-hash-for-PrivateRefCount.patch
> - the "common/hashfn.h" include seems in the wrong place
Fixed in the pushed patch.
> 2) v12-0002-Don-t-allocate-_bt_search-stack.patch
>
> - seems like a reasonable optimization for most callers
> - _bt_search could set "stack_in = new_stack;" in the first if block, I
> think it's nicer
Agreed. Done that way in the commit that I pushed.
> - My only concern is that _bt_search is part of the public API (at least
> technically, as it's in nbtree.h), and we're removing _bt_freestack()
> from the API so the callers won't have a convenient way to free it.
> Maybe that's not an issue in practice, because doing a local version of
> freestack is easy.
I don't think that it's an issue. _bt_search is very much an
nbtree-internal function (amcheck notwithstanding), and as you point
out it's easy enough for an extension to create its own version of
_bt_freestack in the unlikely event that it needs to.
--
Peter Geoghegan
