On Fri, Dec 20, 2024 at 9:55 PM John Naylor <johncnaylo...@gmail.com> wrote: > > On Sat, Dec 21, 2024 at 2:17 AM Masahiko Sawada <sawada.m...@gmail.com> wrote: > > > > On Fri, Dec 20, 2024 at 2:27 AM John Naylor <johncnaylo...@gmail.com> wrote: > > > > v3-0001 allocates the iter data in the caller's context. It's a small > > > patch, but still a core behavior change so proposed for master-only. I > > > believe your v1 is still the least invasive fix for PG17. > > > > I agree to use v1 for v17. > > Okay, did you want to commit that separately, or together with my 0001 > on master? Either way, I've put a bit more effort into the commit > message in v4 and adjusted the comment slightly.
I think we can commit them separately. It's a pure bug fix for back branches while for HEAD it removes the bug by an improvement. > > > > It's trivial after 0001-02: 0003 removes makes one test use slab as > > > the passed context (only 32-bit systems would actually use it > > > currently). > > > > These changes make sense to me. Here are a few comments: > > > > RT_RADIX_TREE has 'leaf_context' but it seems that we use it only for > > local memory. Do we want to declare only in the !RT_SHMEM case? > > That's already the case, if I understand your statement correctly. True. Sorry, I misunderstood something. > > > --- > > /* > > * Similar to TidStoreCreateLocal() but create a shared TidStore on a > > * DSA area. The TID storage will live in the DSA area, and the memory > > * context rt_context will have only meta data of the radix tree. > > * > > * The returned object is allocated in backend-local memory. > > */ > > > > We need to update the comment about rt_context too since we no longer > > use rt_context for shared tidstore. > > Fixed. The three patches look good to me. > BTW, it seems TidStore.context is unused? Indeed. We can remove it. Regards, -- Masahiko Sawada Amazon Web Services: https://aws.amazon.com
