On Tue, Mar 5, 2024 at 6:41 PM John Naylor <johncnaylorls@gmail.com> wrote:
>
> On Tue, Feb 6, 2024 at 9:58 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> >
> > On Fri, Feb 2, 2024 at 8:47 PM John Naylor <johncnaylorls@gmail.com> wrote:
>
> > > It's pretty hard to see what test_pattern() is doing, or why it's
> > > useful. I wonder if instead the test could use something like the
> > > benchmark where random integers are masked off. That seems simpler. I
> > > can work on that, but I'd like to hear your side about test_pattern().
> >
> > Yeah, test_pattern() is originally created for the integerset so it
> > doesn't necessarily fit the radixtree. I agree to use some tests from
> > benchmarks.
>
> Done in v66-0009. I'd be curious to hear any feedback. I like the
> aspect that the random numbers come from a different seed every time
> the test runs.
The new tests look good. Here are some comments:
---
+ expected = keys[i];
+ iterval = rt_iterate_next(iter, &iterkey);
- ndeleted++;
+ EXPECT_TRUE(iterval != NULL);
+ EXPECT_EQ_U64(iterkey, expected);
+ EXPECT_EQ_U64(*iterval, expected);
Can we verify that the iteration returns keys in ascending order?
---
+ /* reset random number generator for deletion */
+ pg_prng_seed(&state, seed);
Why is resetting the seed required here?
---
The radix tree (and dsa in TSET_SHARED_RT case) should be freed at the end.
---
radixtree_ctx = AllocSetContextCreate(CurrentMemoryContext,
"test_radix_tree",
ALLOCSET_DEFAULT_SIZES);
We use a mix of ALLOCSET_DEFAULT_SIZES and ALLOCSET_SMALL_SIZES. I
think it's better to use either one for consistency.
> I'd like to push 0001 and 0002 shortly, and then do another sweep over
> 0003, with remaining feedback, and get that in so we get some
> buildfarm testing before the remaining polishing work on
> tidstore/vacuum.
Sounds a reasonable plan. 0001 and 0002 look good to me. I'm going to
polish tidstore and vacuum patches and update commit messages.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
