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
Attachments:
v1-0001-Use-float8-datatype-for-percentiles-in-pg_walinsp.patchapplication/octet-stream; name=v1-0001-Use-float8-datatype-for-percentiles-in-pg_walinsp.patchDownload+20-21
v1-0001-Use-double-datatype-for-percentiles-in-pgstattupl.patchapplication/octet-stream; name=v1-0001-Use-double-datatype-for-percentiles-in-pgstattupl.patchDownload+1-2
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.