From: | Ibrar Ahmed <ibrar(dot)ahmad(at)gmail(dot)com> |
---|---|
To: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> |
Cc: | "David G(dot) Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com>, "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-07-08 18:09:28 |
Message-ID: | CALtqXTdVDc=GUHQzd2v38J_nhm1tsiZobKmmyotmVij7TSyhkQ@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Thu, Jul 7, 2022 at 2:41 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> On Thu, Jun 23, 2022 at 2:25 AM Ibrar Ahmed <ibrar(dot)ahmad(at)gmail(dot)com> wrote:
> >
> > On Thu, Jun 23, 2022 at 1:04 AM David G. Johnston <
> david(dot)g(dot)johnston(at)gmail(dot)com> wrote:
> >>
> >> - 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.
> >
>
> That change is in the path node which we anyway not going to target as
> part of this change. We only want to change the display for actual
> rows in Explain Analyze. So, I can't see how the quoted change can
> help in any way.
>
> Agreed removed.
> Few miscellaneous comments:
> ========================
> *
> static FullTransactionId XactTopFullTransactionId =
> {InvalidTransactionId};
> -static int nParallelCurrentXids = 0;
> +static int nParallelCurrentXids = 0;
>
> Removed.
> I don't see why this change is required.
>
> * Can you please add a comment explaining why we are making this
> change for actual rows?
>
Done
>
> * Can you please write a test case unless there is some existing test
> that covers the change by displaying actual rows values in decimal but
> in that case patch should have that changed output test? If you don't
> think we can reliably write such a test then please let me know the
> reason?
>
> I think there are tests, and I have updated the results accordingly.
> --
> With Regards,
> Amit Kapila.
>
--
Ibrar Ahmed
Attachment | Content-Type | Size |
---|---|---|
explain_float_row_v3.patch | application/octet-stream | 6.2 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Ibrar Ahmed | 2022-07-08 18:10:31 | Re: explain analyze rows=%.0f |
Previous Message | Zhihong Yu | 2022-07-08 17:38:32 | Re: Aggregate leads to superfluous projection from the scan |