Re: explain analyze rows=%.0f

On Thu, Jun 23, 2022 at 1:04 AM David G. Johnston <
david.g.johnston@gmail.com> wrote:

> On Wed, Jun 22, 2022 at 12:11 PM Ibrar Ahmed <ibrar.ahmad@gmail.com>
> wrote:
>
>> On Thu, Jun 23, 2022 at 12:01 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>
>>> Robert Haas <robertmhaas@gmail.com> writes:
>>> > On Jun 2, 2009, at 9:41 AM, Simon Riggs <simon@2ndQuadrant.com> wrote:
>>> >> You're right that the number of significant digits already exceeds the
>>> >> true accuracy of the computation. I think what Robert wants to see is
>>> >> the exact value used in the calc, so the estimates can be checked more
>>> >> thoroughly than is currently possible.
>>>
>>> > Bingo.
>>>
>>> Uh, the planner's estimate *is* an integer.  What was under discussion
>>> (I thought) was showing some fractional digits in the case where EXPLAIN
>>> ANALYZE is outputting a measured row count that is an average over
>>> multiple loops, and therefore isn't necessarily an integer.  In that
>>> case the measured value can be considered arbitrarily precise --- though
>>> I think in practice one or two fractional digits would be plenty.
>>>
>>>                         regards, tom lane
>>>
>>>
>>> Hi,
>> I was looking at the TODO list and found that the issue requires
>> a quick fix. Attached is a patch which shows output like this.
>>
>
> Quick code review:
>
> + "actual rows=%.0f loops=%.0f": " rows=%.2f loops=%.0f",
>
> The leading space before the else block "rows" does not belong.
>
> There should be a space after the colon.
>
> Thanks, David for your quick response. I have updated the patch.


> The word "actual" that you are dropping in the else block seems like it
> should belong - it is a header for the entire section not just a modifier
> for the word "rows".  This is evidenced by the timing block verbiage where
> rows is standalone and the word actual comes before time.  In short, only
> the format specifier should change under the current scheme.  Both sections.
>
> - WRITE_FLOAT_FIELD(rows, "%.0f");
> + WRITE_FLOAT_FIELD(rows, "%.2f");
>
> This one looks suspicious, though I haven't dug into the code to see
> exactly what all is being touched.  That it doesn't have an nloops
> condition like everything else stands out.
>
> I was also thinking about that, but I don't see any harm when we
ultimately truncating that decimal
at a latter stage of code in case of loop = 1.


> Tooling that expects an integer is the only downside I see here, but I
> concur that the usability of always showing two decimal places when nloops
> > 1 overcomes any objection I have on those grounds.
>
> David J.
>
>

-- 
Ibrar Ahmed