On Thu, Apr 25, 2024 at 9:50 AM Masahiko Sawada <sawada.m...@gmail.com> wrote: > > > I saw a SIGSEGV there when using tidstore to write a fix for something else. > > Patch attached. > > Great find, thank you for the patch!
+1 (This occurred to me a few days ago, but I was far from my computer.) With the purge function that Noah proposed, I believe we can also get rid of the comment at the top of the .sql test file warning of a maintenance hazard: ..."To avoid adding duplicates, -- each call to do_set_block_offsets() should use different block -- numbers." I found that it doesn't add any measurable time to run the test. > The fix looks good to me. I think we can improve regression tests for > better coverage. In TidStore on a 64-bit machine, we can store 3 > offsets in the header and these values are embedded to the leaf page. > With more than 3 offsets, the value size becomes more than 16 bytes > and a single value leaf. Therefore, if we can add the test with the > array[1,2,3,4,100], we can cover the case of replacing a single-value > leaf with a different size new single-value leaf. Now we add 9 pairs Good idea. > of do_gset_block_offset() and check_set_block_offsets(). If these are > annoying, we can remove the cases of array[1] and array[1,2]. Let's keep those -- 32-bit platforms should also exercise this path.