On Sun, Oct 1, 2017 at 5:18 AM, Nikolay Shaplov <dh...@nataraj.su> wrote: > src/backend/access/common/reloptions.c get only 7 lines, it was quite covered > by existing test, but all most of the access methods gets some coverage > increase: > > src/backend/access/brin 1268 -> 1280 (+18) > src/backend/access/gin 2924 -> 2924 (0) > src/backend/access/gist 1673 -> 2108 (+435) > src/backend/access/hash 1580 -> 1638 (+58) > src/backend/access/heap 2863 -> 2866 (+3) > src/backend/access/nbtree 2565 -> 2647 (+82) > src/backend/access/spgist 2066 -> 2068 (+2) > > Though I should say that incredible coverage boost for gist, is the result of > not steady results of test run. The real value should be much less...
+-- Test buffering and fillfactor reloption takes valid values +create index gist_pointidx2 on gist_point_tbl using gist(p) with (buffering = on, fillfactor=50); +create index gist_pointidx3 on gist_point_tbl using gist(p) with (buffering = off); +create index gist_pointidx4 on gist_point_tbl using gist(p) with (buffering = auto); Those are the important bits doing the boost in gist visibly. To be kept. -CREATE INDEX hash_f8_index ON hash_f8_heap USING hash (random float8_ops); +CREATE INDEX hash_f8_index ON hash_f8_heap USING hash (random float8_ops) WITH (fillfactor=60); Woah. So that alone increases hash by 58 steps. +CREATE INDEX bloomidx2 ON tst USING bloom (i, t) WITH (length=0); +ERROR: value 0 out of bounds for option "length" +DETAIL: Valid values are between "1" and "4096". contrib/bloom contributes to the coverage of reloptions.c thanks to its coverage of the add_ routines when the library is loaded. And those don't actually improve any coverage, right? > Nevertheless tests touches the reloptions related code, checks for proper > error handling, and it is good. Per your measurements reloptions.c is improved by 1.7%, or 7 lines as you say. Five of them are in parse_one_reloption for integer (2) and reals (2), plus one error at the top. Two are in transformRelOptions for a valid namespace handling. In your patch reloptions.sql is 141 lines. Couldn't it be shorter with the same improvements? It looks that a lot of tests are overlapping with existing ones. > I think we should commit it. My take is that a lighter version could be committed instead. My suggestion is to keep the new test reloptions minimal so as it tests the relopt types and their bounds, and I think that you could remove the same bounding checks in the other tests that you added: bloom, gist, etc. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers