Обсуждение: pgsql: Improve BRIN minmax-multi opclass test coverage
Improve BRIN minmax-multi opclass test coverage Per the code coverage report, the existing regression tests did not exercice some a couple important BRIN minmax-multi code paths. - The tests focused on testing planning with a range of scan key strategies, but not the execution. Fixed by adding queries that actually test query execution for both equality and inequality. - All tests created indexes after inserting data, but this only exercises the CREATE INDEX strategy that sees all values at once, not incremental summary updates. The new tests flip the order and create the index before adding data. - The assert check(s) validating correctness of expanded ranges were present only in the "union" code path, which is not covered by regression tests at all (as it requires concurrency etc.). Fixed by adding the asserts to a couple more places. Reviewed-by: Heikki Linnakangas Discussion: https://postgr.es/m/57020b2e-d9c9-9bc7-4892-b36d9bb07563%40enterprisedb.com Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/0457109344b46f481f9bf09b85674695ba57c8e4 Modified Files -------------- src/backend/access/brin/brin_minmax_multi.c | 9 + src/test/regress/expected/brin_multi.out | 357 ++++++++++++++++++++++++++++ src/test/regress/sql/brin_multi.sql | 165 +++++++++++++ 3 files changed, 531 insertions(+)
These new tests introduced a few new calls of the md5() function. This should be avoided (see commit 208bf364a9). You can replace these easily with the fipshash() function instead, but I think you then also need to change some of the constants in the tests. Could you look at this again? On 02.07.23 10:34, Tomas Vondra wrote: > Improve BRIN minmax-multi opclass test coverage > > Per the code coverage report, the existing regression tests did not > exercice some a couple important BRIN minmax-multi code paths. > > - The tests focused on testing planning with a range of scan key > strategies, but not the execution. Fixed by adding queries that > actually test query execution for both equality and inequality. > > - All tests created indexes after inserting data, but this only > exercises the CREATE INDEX strategy that sees all values at once, not > incremental summary updates. The new tests flip the order and create > the index before adding data. > > - The assert check(s) validating correctness of expanded ranges were > present only in the "union" code path, which is not covered by > regression tests at all (as it requires concurrency etc.). Fixed by > adding the asserts to a couple more places. > > Reviewed-by: Heikki Linnakangas > Discussion: https://postgr.es/m/57020b2e-d9c9-9bc7-4892-b36d9bb07563%40enterprisedb.com > > Branch > ------ > master > > Details > ------- > https://git.postgresql.org/pg/commitdiff/0457109344b46f481f9bf09b85674695ba57c8e4 > > Modified Files > -------------- > src/backend/access/brin/brin_minmax_multi.c | 9 + > src/test/regress/expected/brin_multi.out | 357 ++++++++++++++++++++++++++++ > src/test/regress/sql/brin_multi.sql | 165 +++++++++++++ > 3 files changed, 531 insertions(+) >
On 22.08.23 08:43, Peter Eisentraut wrote: > These new tests introduced a few new calls of the md5() function. This > should be avoided (see commit 208bf364a9). You can replace these easily > with the fipshash() function instead, but I think you then also need to > change some of the constants in the tests. Could you look at this again? I have created a patch that updates the test accordingly. It replaces the md5() function with fipshash() and adjusts the test values to correspond to the values generated in the original tests.