On 14.06.23 20:41, Corey Huinker wrote:
So maybe we should make table_block_relation_estimate_size smarter to also consider the fillfactor in the "no statistics" branch, per the attached patch.I like this a lot. The reasoning is obvious, the fix is simple,it doesn't upset any make-check-world tests, and in order to get a performance regression we'd need a table whose fillfactor has been changed after the data was loaded but before an analyze happens, and that's a narrow enough case to accept.My only nitpick is to swap (usable_bytes_per_page * fillfactor / 100) / tuple_width with (usable_bytes_per_page * fillfactor) / (tuple_width * 100)as this will eliminate the extra remainder truncation, and it also gets the arguments "in order" algebraically.
The fillfactor is in percent, so it makes sense to divide it by 100 first before doing any further arithmetic with it. I find your version of this to be more puzzling without any explanation. You could do fillfactor/100.0 to avoid the integer division, but then, the comment above that says "integer division is intentional here", without any further explanation. I think a bit more explanation of all the subtleties here would be good in any case.
