On Mon, Aug 28, 2023 at 4:20 PM John Naylor <john.nay...@enterprisedb.com> wrote: > > On Sun, Aug 27, 2023 at 7:53 PM Masahiko Sawada <sawada.m...@gmail.com> wrote: > > > > I've updated the regression tests for tidstore so that it uses SQL > > functions to add blocks/offsets and dump its contents. The new test > > covers the same test coverages but it's executed using SQL functions > > instead of executing all tests in one SQL function. > > This is much nicer and more flexible, thanks! A few questions/comments: > > tidstore_dump_tids() returns a string -- is it difficult to turn this into a > SRF, or is it just a bit more work?
It's not difficult. I've changed it in v42 patch. > > The lookup test seems fine for now. The output would look nicer with an > "order by tid". Agreed. > > I think we could have the SQL function tidstore_create() take a boolean for > shared memory. That would allow ad-hoc testing without a recompile, if I'm > not mistaken. Agreed. > > +SELECT tidstore_set_block_offsets(blk, array_agg(offsets.off)::int2[]) > + FROM blocks, offsets > + GROUP BY blk; > + tidstore_set_block_offsets > +---------------------------- > + > + > + > + > + > +(5 rows) > > Calling a void function multiple times leads to vertical whitespace, which > looks a bit strange and may look better with some output, even if irrelevant: > > -SELECT tidstore_set_block_offsets(blk, array_agg(offsets.off)::int2[]) > +SELECT row_number() over(order by blk), tidstore_set_block_offsets(blk, > array_agg(offsets.off)::int2[]) > > row_number | tidstore_set_block_offsets > ------------+---------------------------- > 1 | > 2 | > 3 | > 4 | > 5 | > (5 rows) Yes, it looks better. I've attached v42 patch set. I improved tidstore regression test codes in addition of imcorporating the above comments. Regards, -- Masahiko Sawada Amazon Web Services: https://aws.amazon.com
v42-ART.tar.gz
Description: GNU Zip compressed data
