Clarify the logic in a few places in the new balanced merge code.
authorHeikki Linnakangas <heikki.linnakangas@iki.fi>
Mon, 25 Oct 2021 06:30:49 +0000 (09:30 +0300)
committerHeikki Linnakangas <heikki.linnakangas@iki.fi>
Mon, 25 Oct 2021 06:30:49 +0000 (09:30 +0300)
commit166f94377c886516ca986ef8a623cd2e854fe911
treec599d32539e8d0cc5f0a4dd4c3aed9b43146d256
parentb4ada4e19fd7bedb433e46516ccd0ca4213d2719
Clarify the logic in a few places in the new balanced merge code.

In selectnewtape(), use 'nOutputTapes' rather than 'nOutputRuns' in the
check for whether to start a new tape or to append a new run to an
existing tape. Until 'maxTapes' is reached, nOutputTapes is always equal
to nOutputRuns, so it doesn't change the logic, but it seems more logical
to compare # of tapes with # of tapes. Also, currently maxTapes is never
modified after the merging begins, but written this way, the code would
still work if it was. (Although the nOutputRuns == nOutputTapes assertion
would need to be removed and using nOutputRuns % nOutputTapes to
distribute the runs evenly across the tapes wouldn't do a good job
anymore).

Similarly in mergeruns(), change to USEMEM(state->tape_buffer_mem) to
account for the memory used for tape buffers. It's equal to availMem
currently, but tape_buffer_mem is more direct and future-proof. For
example, if we changed the logic to only allocate half of the remaining
memory to tape buffers, USEMEM(state->tape_buffer_mem) would still be
correct.

Coverity complained about these. Hopefully this patch helps it to
understand the logic better. Thanks to Tom Lane for initial analysis.
src/backend/utils/sort/tuplesort.c