On Sun, Apr 14, 2024 at 6:33 PM Andres Freund <and...@anarazel.de> wrote: > - Some of the new nbtree code could use a bit more tests: > > https://anarazel.de/postgres/cov/16-vs-HEAD-2024-04-14/src/backend/access/nbtree/nbtutils.c.gcov.html#L1468
I made a conscious decision to not add coverage for the function that you've highlighted here (_bt_rewind_nonrequired_arrays) back when I reviewed the coverage situation for the patch, which was about a month ago now. (FWIW I also decided against adding coverage for the recursive call to _bt_advance_array_keys, for similar reasons.) I don't mind adding _bt_rewind_nonrequired_arrays coverage now. I already wrote 4 tests that show wrong answers (and assertion failures) when the call to _bt_rewind_nonrequired_arrays is temporarily commented out. The problem with committing such a test, if any, is that it'll necessitate creating an index with at least 3 columns, crafted to trip up this exact issue with non-required arrays -- and it has to be bigger than one page (probably several pages, at a minimum). That's a relatively large number of cycles to spend on this fairly narrow issue -- it's at least a lot relative to the prevailing standard for these things. Plus I'd be relying on implementation details that might change, as well as relying on things like BLCKSZ (not that it'd be the first time that I committed a test like that). Note also that there is a general rule (explained above _bt_rewind_nonrequired_arrays) requiring that all non-required arrays be reset to their initial positions/element (first in the current scan direction) once _bt_first is reached. If _bt_advance_array_keys somehow failed to follow that rule (not necessarily due to an issue with missing the call to _bt_rewind_nonrequired_arrays), then we'd get an assertion failure within _bt_first -- the _bt_verify_arrays_bt_first assertion would catch the violation of the invariant (the_bt_advance_array_keys postcondition invariant/_bt_first precondition invariant). So we kinda do have some test coverage for this function already. -- Peter Geoghegan
