On Thu, Dec 21, 2023 at 10:19 AM John Naylor <johncnaylorls@gmail.com> wrote:
>
> On Wed, Dec 20, 2023 at 6:36 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> >
> > I've updated the new patch set that incorporated comments I got so
> > far. 0007, 0008, and 0012 patches are updates from the v45 patch set.
> > In addition to the review comments, I made some changes in tidstore to
> > make it independent from heap. Specifically, it uses MaxOffsetNumber
> > instead of MaxHeapTuplesPerPage. Now we don't need to include
> > htup_details.h. It enlarged MaxBlocktableEntrySize but it's still 272
> > bytes.
>
> That's a good idea.
>
> > BTW regarding the previous comment I got before:
> >
> > > - RT_PTR_ALLOC *slot;
> > > + RT_PTR_ALLOC *slot = NULL;
> > >
> > > We have a macro for invalid pointer because of DSA.
> >
> > I think that since *slot is a pointer to a RT_PTR_ALLOC it's okay to set NULL.
>
> Ah right, it's the address of the slot.
>
> > I'm going to update RT_DUMP() and RT_DUMP_NODE() codes for the next step.
>
> That could probably use some discussion. A few months ago, I found the
> debugging functions only worked when everything else worked. When
> things weren't working, I had to rip one of these functions apart so
> it only looked at one node. If something is broken, we can't count on
> recursion or iteration working, because we won't get that far. I don't
> remember how things are in the current patch.
Agreed.
I found the following comment and wanted to discuss:
// this might be better as "iterate over nodes", plus a callback to
RT_DUMP_NODE,
// which should really only concern itself with single nodes
RT_SCOPE void
RT_DUMP(RT_RADIX_TREE *tree)
If it means we need to somehow use the iteration functions also for
dumping the whole tree, it would probably need to refactor the
iteration codes so that the RT_DUMP() can use them while dumping
visited nodes. But we need to be careful of not adding overheads to
the iteration performance.
>
> I've finished the node shrinking and addressed some fixme/todo areas
> -- can I share these and squash your v46 changes first?
Cool! Yes, please do so.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com