Horton

Thread

Commits

  1. pgstattuple: Use double consistently for percentages

  2. Use float8 datatype for percentiles in pg_walinspect stat functions

  1. pg_walinspect float4/float8 confusion

    Peter Eisentraut <peter.eisentraut@enterprisedb.com> — 2022-09-08T11:53:10Z

    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.

  2. Re: pg_walinspect float4/float8 confusion

    Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> — 2022-09-09T03:51:17Z

    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

  3. Re: pg_walinspect float4/float8 confusion

    Peter Eisentraut <peter.eisentraut@enterprisedb.com> — 2022-09-12T08:28:38Z

    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.