MIME-Version: 1.0
References: <603c8f070905281830g2e5419c4xad2946d149e21f9d@mail.gmail.com>
 <4A1F4FD5.7060204@timbira.com> <4A249CE9.6050708@cheapcomplexdevices.com>
 <1243950060.23910.58.camel@ebony.2ndQuadrant>
 <B8F900A1-1ACB-4C84-84D6-676B2B7DF8A2@gmail.com>
 <11926.1243953507@sss.pgh.pa.us>
 <CALtqXTeQ+QCFT0DWcM5LOjnf9_Fz59+ALFejNEFwpFhW-9o4ag@mail.gmail.com>
 <CAKFQuwYg+HtK+zFKENbsrLgz9x8LL1HWau6DR7Om6o1dT=tt6g@mail.gmail.com>
 <CALtqXTcrXtwZ3RRg2FH8yt8mDxRypXrUbw6a=zp0x2fKm_5xNg@mail.gmail.com>
In-Reply-To: 
 <CALtqXTcrXtwZ3RRg2FH8yt8mDxRypXrUbw6a=zp0x2fKm_5xNg@mail.gmail.com>
From: vignesh C <vignesh21@gmail.com>
Date: Thu, 7 Jul 2022 15:44:37 +0530
Message-ID: 
 <CALDaNm3inETBcHYyBeevfKCjaxB24N4BVhDiZVdBooEdtxi1_w@mail.gmail.com>
Subject: Re: explain analyze rows=%.0f
To: Ibrar Ahmed <ibrar.ahmad@gmail.com>
Cc: "David G. Johnston" <david.g.johnston@gmail.com>,
	"pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>,
 Robert Haas <robertmhaas@gmail.com>,
	Simon Riggs <simon@2ndquadrant.com>,
 Ron Mayer <rm_pg@cheapcomplexdevices.com>,
	Euler Taveira de Oliveira <euler@timbira.com>, Tom Lane <tgl@sss.pgh.pa.us>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: 
 <https://www.postgresql.org/message-id/CALDaNm3inETBcHYyBeevfKCjaxB24N4BVhDiZVdBooEdtxi1_w%40mail.gmail.com>
Precedence: bulk

On Thu, Jun 23, 2022 at 2:25 AM Ibrar Ahmed <ibrar.ahmad@gmail.com> wrote:
>
>
>
> 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> wro=
te:
>>>
>>> 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> wrot=
e:
>>>> >> 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 m=
ore
>>>> >> 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 EXPLA=
IN
>>>> 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 --- thou=
gh
>>>> 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=3D%.0f loops=3D%.0f": " rows=3D%.2f loops=3D%.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 f=
or the word "rows".  This is evidenced by the timing block verbiage where r=
ows is standalone and the word actual comes before time.  In short, only th=
e 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 exa=
ctly what all is being touched.  That it doesn't have an nloops condition l=
ike everything else stands out.
>>
> I was also thinking about that, but I don't see any harm when we ultimate=
ly truncating that decimal
> at a latter stage of code in case of loop =3D 1.

Thanks for the patch.

1) There are some existing regression tests that are failing, you
should update the expect files accordingly for the same:
--- /home/vignesh/postgres/src/test/regress/expected/select_parallel.out
       2022-05-18 20:51:46.874818044 +0530
+++ /home/vignesh/postgres/src/test/regress/results/select_parallel.out
2022-07-07 15:27:34.450440922 +0530
@@ -545,17 +545,17 @@
 explain (analyze, timing off, summary off, costs off)
    select count(*) from tenk1, tenk2 where tenk1.hundred > 1
         and tenk2.thousand=3D0;
-                                QUERY PLAN
---------------------------------------------------------------------------
+                                 QUERY PLAN
+--------------------------------------------------------------------------=
---
  Aggregate (actual rows=3D1 loops=3D1)
    ->  Nested Loop (actual rows=3D98000 loops=3D1)
          ->  Seq Scan on tenk2 (actual rows=3D10 loops=3D1)
                Filter: (thousand =3D 0)
                Rows Removed by Filter: 9990
-         ->  Gather (actual rows=3D9800 loops=3D10)
+         ->  Gather (actual rows=3D9800.00 loops=3D10)
                Workers Planned: 4
                Workers Launched: 4
-               ->  Parallel Seq Scan on tenk1 (actual rows=3D1960 loops=3D=
50)
+               ->  Parallel Seq Scan on tenk1 (actual rows=3D1960.00 loops=
=3D50)
                      Filter: (hundred > 1)

    test select_parallel              ... FAILED      744 ms
     partition_prune              ... FAILED      861 ms
     explain                      ... FAILED      134 ms
     memoize                      ... FAILED      250 ms

2) This change is not required as part of this patch:
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -122,7 +122,7 @@ bool                bsysscan =3D false;
  * lookups as fast as possible.
  */
 static FullTransactionId XactTopFullTransactionId =3D {InvalidTransactionI=
d};
-static int nParallelCurrentXids =3D 0;
+static int     nParallelCurrentXids =3D 0;
 static TransactionId *ParallelCurrentXids;

Regards,
Vignesh