Re: explain analyze rows=%.0f

From: Ibrar Ahmed <ibrar(dot)ahmad(at)gmail(dot)com>
To: "David G(dot) Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com>
Cc: "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>, Robert Haas <robertmhaas(at)gmail(dot)com>, Simon Riggs <simon(at)2ndquadrant(dot)com>, Ron Mayer <rm_pg(at)cheapcomplexdevices(dot)com>, Euler Taveira de Oliveira <euler(at)timbira(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Subject: Re: explain analyze rows=%.0f
Date: 2022-06-22 20:55:00
Message-ID: CALtqXTcrXtwZ3RRg2FH8yt8mDxRypXrUbw6a=zp0x2fKm_5xNg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

> On Wed, Jun 22, 2022 at 12:11 PM Ibrar Ahmed <ibrar(dot)ahmad(at)gmail(dot)com>
> wrote:
>
>> On Thu, Jun 23, 2022 at 12:01 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>
>>> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>>> > On Jun 2, 2009, at 9:41 AM, Simon Riggs <simon(at)2ndQuadrant(dot)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

Attachment Content-Type Size
explain_float_row_v2.patch application/octet-stream 3.4 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Bagga, Rishu 2022-06-22 21:06:29 SLRUs in the main buffer pool - Page Header definitions
Previous Message Robert Haas 2022-06-22 20:30:36 Re: replacing role-level NOINHERIT with a grant-level option