Re: tuplesort test coverage

On Sun, Oct 13, 2019 at 3:41 PM Andres Freund <andres(at)anarazel(dot)de> wrote:
> - cluster for expression indexes (line 935)

We've never had coverage of this, but perhaps that can be added now.

> - sorts exceeding INT_MAX / 2 memory (line 1337), but that seems hard to
> test realistically

I don't think that that can be tested, realistically.

> - aborted abbreviated keys (lines 1522, 1608, 1774, 3620, 3739, 3867, 4266)

Also hard to test -- there was a bug here when abbreviated keys first
went in -- that was detected by amcheck.

All of the places where we abort are essentially the same, though.

> - in memory backwards scans (lines 1936, 3042)
> - *any* coverage for TSS_SORTEDONTAPE (line 1964)

That used to exist, but it went away when we killed replacement selection sort.

> - disk sort skiptuples (line 2325)

Couldn't hurt.

> - mergeruns without abbrev key (line 2582)

mergeruns() doesn't use abbreviated keys -- this code disables their
use in the standard way.

> - disk sorts with more than one run (lines 2707, 2789)
> - any disk based tuplesort_begin_heap() (lines 3649, 3676)

I had to convince Tom to get the coverage of external sorts we have
now. Apparently there are buildfarm animals that are very sensitive to
that cost, that could have substantially increased test runtimes were
we to do more. Perhaps this could be revisited.

> - Seems copytup_index currently is essentially dead, because
> tuplesort_putindextuplevalues() doesn't use COPYTUP (line 4142)

Yeah, that looks like dead code -- it should just be a stub with a
"can't happen" error.

> I'm pretty unhappy that tuplesort has been whacked around pretty heavily
> in the last few years, while *reducing* effective test coverage
> noticeably, rather than increasing it.

I don't think that that's true, on balance. There are only 1,000 extra
lines of code in tuplesort.c in master compared to 9.4, even though we
added parallel sorts and abbreviated keys, two huge enhancements. In
many ways, tuplesort is now simpler than ever.

> There's pretty substantial and
> nontrivial areas without any tests - do we have actually have any
> confidence that they work?

Everything that you're talking about has existed since v11 came out a
year ago, and most of it is a year or two older than that. So yeah,
I'm pretty confident that it works.

--
Peter Geoghegan