From: | Peter Eisentraut <peter(at)eisentraut(dot)org> |
---|---|
To: | Corey Huinker <corey(dot)huinker(at)gmail(dot)com>, Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com> |
Cc: | PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: Should heapam_estimate_rel_size consider fillfactor? |
Date: | 2023-07-03 06:46:00 |
Message-ID: | 63729d7a-cb1e-06af-a24e-27a8b93717ef@eisentraut.org |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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.
From | Date | Subject | |
---|---|---|---|
Next Message | Peter Eisentraut | 2023-07-03 06:48:22 | Re: Fix regression tests to work with REGRESS_OPTS=--no-locale |
Previous Message | Peter Eisentraut | 2023-07-03 06:34:39 | Re: Add information about command path and version of flex in meson output |