Thread: pgstattuple: add test for coverage
Hi, hackers I added some SQL statements to improve test coverage. As data was inserted, the expected file changed. So should I change all `select *` for a stable expected result? And it's the coverage change as I add 50.6% -> 78.7% --- regards, Lee Dong Wook
Attachment
Dong Wook Lee <sh95119@gmail.com> writes:
> Hi, hackers
> I added some SQL statements to improve test coverage.
I do not think it's a great idea to create random dependencies
between modules like the pgstattuple -> bloom dependency you
casually added here.
regards, tom lane
Tom Lane <tgl@sss.pgh.pa.us> writes: > I do not think it's a great idea to create random dependencies > between modules like the pgstattuple -> bloom dependency you > casually added here. I agree with your option. Is there no problem with selecting all the columns during SELECT statements? I thought there might be a problem where the test results could change easily. --- regards Lee Dong Wook.
Attachment
Hi, On 2022-08-03 11:19:59 +0900, Dong Wook Lee wrote: > Is there no problem with selecting all the columns during SELECT statements? > I thought there might be a problem where the test results could change easily. Which indeed is the case, e.g. on 32bit systems it fails: https://cirrus-ci.com/task/4619535222308864?logs=test_world_32#L253 https://api.cirrus-ci.com/v1/artifact/task/4619535222308864/testrun/build-32/testrun/pgstattuple/regress/regression.diffs table_len | tuple_count | tuple_len | tuple_percent | dead_tuple_count | dead_tuple_len | dead_tuple_percent | free_space| free_percent -----------+-------------+-----------+---------------+------------------+----------------+--------------------+------------+-------------- - 1171456 | 5000 | 560000 | 47.8 | 5000 | 560000 | 47.8 | 7452| 0.64 + 1138688 | 5000 | 540000 | 47.42 | 5000 | 540000 | 47.42 | 14796| 1.3 (1 row) ... You definitely can't rely on such details not to change across platforms. Greetings, Andres Freund
> Which indeed is the case, e.g. on 32bit systems it fails: > > https://cirrus-ci.com/task/4619535222308864?logs=test_world_32#L253 > > https://api.cirrus-ci.com/v1/artifact/task/4619535222308864/testrun/build-32/testrun/pgstattuple/regress/regression.diffs > > table_len | tuple_count | tuple_len | tuple_percent | dead_tuple_count | dead_tuple_len | dead_tuple_percent | free_space| free_percent > -----------+-------------+-----------+---------------+------------------+----------------+--------------------+------------+-------------- > - 1171456 | 5000 | 560000 | 47.8 | 5000 | 560000 | 47.8 | 7452 | 0.64 > + 1138688 | 5000 | 540000 | 47.42 | 5000 | 540000 | 47.42 | 14796| 1.3 > (1 row) > > ... > > > You definitely can't rely on such details not to change across platforms. Thank you for letting me know I'll fix it and check if there's any problem.
Hi, On 2022-10-03 00:42:27 +0900, Dong Wook Lee wrote: > > Which indeed is the case, e.g. on 32bit systems it fails: > > > > https://cirrus-ci.com/task/4619535222308864?logs=test_world_32#L253 > > > > https://api.cirrus-ci.com/v1/artifact/task/4619535222308864/testrun/build-32/testrun/pgstattuple/regress/regression.diffs > > > > table_len | tuple_count | tuple_len | tuple_percent | dead_tuple_count | dead_tuple_len | dead_tuple_percent | free_space| free_percent > > -----------+-------------+-----------+---------------+------------------+----------------+--------------------+------------+-------------- > > - 1171456 | 5000 | 560000 | 47.8 | 5000 | 560000 | 47.8 | 7452 | 0.64 > > + 1138688 | 5000 | 540000 | 47.42 | 5000 | 540000 | 47.42 | 14796 | 1.3 > > (1 row) > > > > ... > > > > > > You definitely can't rely on such details not to change across platforms. > Thank you for letting me know I'll fix it and check if there's any problem. I've marked the patch as returned with feedback for now. Please change that once you submit an updated version. Greetings, Andres Freund