On Mon, 17 Mar 2025 at 23:51, Matthias van de Meent <boekewurm+postg...@gmail.com> wrote: > > On Tue, 11 Mar 2025 at 16:53, Peter Geoghegan <p...@bowt.ie> wrote: > > > > On Sat, Mar 8, 2025 at 11:43 AM Peter Geoghegan <p...@bowt.ie> wrote: > > > I plan on committing this one soon. It's obviously pretty pointless to > > > make the BTMaxItemSize operate off of a page header, and not requiring > > > it is more flexible. > > > > Committed. And committed a revised version of "Show index search count > > in EXPLAIN ANALYZE" that addresses the issues with non-parallel-aware > > index scan executor nodes that run from a parallel worker. > > > > Attached is v28. This is just to keep the patch series applying > > cleanly -- no real changes here. > > You asked off-list for my review of 0003. I'd already reviewed 0001 > before that, so that review also included. I'll see if I can spend > some time on the other patches too
My comments on 0004: > _bt_skiparray_strat_decrement > _bt_skiparray_strat_increment In both functions the generated value isn't used when the in/decrement overflows (and thus invalidates the qual), or when the opclass somehow doesn't have a <= or >= operator, respectively. For byval types that's not much of an issue, but for by-ref types (such as uuid, or bigint on 32-bit systems) that's not great, as btree explicitly allows no leaks for the in/decrement functions, and now we use those functions and leak the values. Additionally, the code is essentially duplicated between the functions, with as only differences which sksup function to call; which opstrategies to check, and where to retrieve/put the value. It's only 2 instances total, but if you figure out how to make a nice single function from the two that'd be appreciated, as it reduces duplication and chances for divergence. Kind regards, Matthias van de Meent Neon (https://neon.tech)
