On Thu, Mar 12, 2026 at 10:43 AM Tomas Vondra <[email protected]> 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


Reply via email to