Re: [PATCH] Add tests for Bitmapset
От | Michael Paquier |
---|---|
Тема | Re: [PATCH] Add tests for Bitmapset |
Дата | |
Msg-id | aNEBUeYqqSVOSVaP@paquier.xyz обсуждение исходный текст |
Ответ на | Re: [PATCH] Add tests for Bitmapset (Greg Burd <greg@burd.me>) |
Ответы |
Re: [PATCH] Add tests for Bitmapset
Re: [PATCH] Add tests for Bitmapset |
Список | pgsql-hackers |
On Fri, Sep 19, 2025 at 02:48:43PM -0400, Greg Burd wrote: > I think I've incorporated your feedback which did indeed make the patch > more readable/crisp and maintainable, thank you. > > coverage: HEAD v7 v8 > lines......: 87.1 90.5 92.2 bitmap_match() had some duplicated tests. bms_replace_members() was not covered, and a function was added to the module. I get up to 93.4% for the lines, once added. +PG_FUNCTION_INFO_V1(test_bms_from_array); +PG_FUNCTION_INFO_V1(test_bms_to_array); These two were still declared, are not required. There were a couple of functions that acted as dead code in the module, covered in the tree. As we are trying to cover the gap I think that we should just do the whole thing. Here are the functions: - is_empty - subset_compare - get_singleton_member - nonempty_difference - member_index - join. The test C function had a mistake, because either input may be modified or updated by bms_join(). The code was freeing the right input, leading to errors in the node output function when tested. Mixing the CTEs and the UNION ALL would have been nice if the queries in the UNIONs relied on more than one input value, but none of them did that, so I have removed this part and made the tests leaner. That feels easier for the eye. Testing on equality for the hash functions should be OK, so I have kept these, including the NULL/0 cases. A few incorrect things related to the style, fixed on the way, like an incorrect copyright notice in meson.build, the top of test_bitmapset.c was largely incorrect. It seems like these were copy-pasted from elsewhere. Another thing was testing with -DREALLOCATE_BITMAPSETS, which I've done. A couple of code paths related to the test C functions don't test for NULL input, which is still OK as these don't impact the internal tests or the backend coverage, so I've left these out. The result I had was good enough, so applied. The CI was OK, the buildfarm may have a different opinion. -- Michael
Вложения
В списке pgsql-hackers по дате отправления: