On Fri, Jul 1, 2016 at 7:05 PM, Noah Misch <n...@leadboat.com> wrote: > I think such a test would suffice to cover this bug if Valgrind Memcheck does > detect the problem in it.
Are you asking me to produce a regression test that exercises the bug? I would be happy to do so, but I require some clarification on project policy first. As I already mentioned, we currently have *precisely* zero test coverage of external sorting. Apparently, avoiding external sort operations in the regression tests has value. I attempted to address this with amcheck, which had extensive tests for B-Tree index builds in its standard regression tests (including coverage of collatable types; testing using amcheck conveniently made the tests portable). The theory there was that as a contrib module, amcheck's regression tests could be run less frequently by hackers, possibly very infrequently by having a special test_decoding style target. Unfortunately, no committer took an interest in amcheck at the time, so that has yet to go anywhere. If things had gone differently there, it would be pretty straightforward to add a few CLUSTER commands to those tests. Still, it probably makes sense to have a dedicated tuplesort regression test suite, that is (through whatever mechanism) only run selectively on certain buildfarm animals that don't have slow I/O subsystems. The tests must also be easily avoidable by hackers when running tests on their development machines. I think that a new tuplesort.sql test file is where a test like this belongs (not in the existing cluster.sql test file). If I write a patch along those lines, is it ever going to be accepted? I would be happy to work on this, but we need to have a sensible conversation on what's acceptable to everyone in this area first. I've screamed bloody murder about the test coverage in this area before, and nothing happened. If you think I'm overstating things, then consider how many commits that touched tuplesort.c added any tests. Even the commit "Fix inadequately-tested code path in tuplesort_skiptuples()." didn't add tests! There is scarcely any example of anyone deliberately adding test coverage to that file. I don't understand why it is so. -- Peter Geoghegan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
