Thread: pg_walinspect float4/float8 confusion
The pg_walinspect function pg_get_wal_stats() has output arguments declared as float4 (count_percentage, record_size_percentage, etc.), but the internal computations are all done in type double. Is there a reason why this is then converted to float4 for output? It probably doesn't matter in practice, but it seems unnecessarily confusing. Or at least add a comment so it doesn't look like an accident. Also compare with pgstattuple, which uses float8 in its SQL interface for similar data.
On Thu, Sep 8, 2022 at 5:23 PM Peter Eisentraut <peter.eisentraut@enterprisedb.com> wrote: > > The pg_walinspect function pg_get_wal_stats() has output arguments > declared as float4 (count_percentage, record_size_percentage, etc.), but > the internal computations are all done in type double. Is there a > reason why this is then converted to float4 for output? It probably > doesn't matter in practice, but it seems unnecessarily confusing. Or at > least add a comment so it doesn't look like an accident. Also compare > with pgstattuple, which uses float8 in its SQL interface for similar data. Thanks for finding this. There's no specific reason as such. However, it's good to be in sync with what code does internally and what it exposes to the users. pg_walinspect uses double data type (double precision floating point number) for internal calculations and cuts it down to single precision floating point number float4 to the users. Attaching a patch herewith. I'm not sure if this needs to be backported, if at all, we were to, IMO it should be backported to reduce the code diff. While on, I found that pgstattuple uses uint64 for internal percentile calculations as opposed to double data type for others. Attaching a small patch to fix it. -- Bharath Rupireddy PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Attachment
On 09.09.22 05:51, Bharath Rupireddy wrote: > On Thu, Sep 8, 2022 at 5:23 PM Peter Eisentraut > <peter.eisentraut@enterprisedb.com> wrote: >> >> The pg_walinspect function pg_get_wal_stats() has output arguments >> declared as float4 (count_percentage, record_size_percentage, etc.), but >> the internal computations are all done in type double. Is there a >> reason why this is then converted to float4 for output? It probably >> doesn't matter in practice, but it seems unnecessarily confusing. Or at >> least add a comment so it doesn't look like an accident. Also compare >> with pgstattuple, which uses float8 in its SQL interface for similar data. > > Thanks for finding this. There's no specific reason as such. However, > it's good to be in sync with what code does internally and what it > exposes to the users. pg_walinspect uses double data type (double > precision floating point number) for internal calculations and cuts it > down to single precision floating point number float4 to the users. > Attaching a patch herewith. I'm not sure if this needs to be > backported, if at all, we were to, IMO it should be backported to > reduce the code diff. done > While on, I found that pgstattuple uses uint64 for internal percentile > calculations as opposed to double data type for others. Attaching a > small patch to fix it. Good find. I also changed the computation to use 100.0 instead of 100, so that you actually get non-integer values out of it. I didn't backpatch this, since it would probably result in a small behavior change and the results with the previous code are not wrong, just unnecessarily truncated.