On Thu, Dec 21, 2023 at 10:19 AM John Naylor <johncnaylo...@gmail.com> wrote: > > On Wed, Dec 20, 2023 at 6:36 PM Masahiko Sawada <sawada.m...@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