public inbox for [email protected]
help / color / mirror / Atom feedFrom: Ibrar Ahmed <[email protected]>
To: David G. Johnston <[email protected]>
Cc: [email protected] <[email protected]>
Cc: Robert Haas <[email protected]>
Cc: Simon Riggs <[email protected]>
Cc: Ron Mayer <[email protected]>
Cc: Euler Taveira de Oliveira <[email protected]>
Cc: Tom Lane <[email protected]>
Subject: Re: explain analyze rows=%.0f
Date: Thu, 23 Jun 2022 01:55:00 +0500
Message-ID: <CALtqXTcrXtwZ3RRg2FH8yt8mDxRypXrUbw6a=zp0x2fKm_5xNg@mail.gmail.com> (raw)
In-Reply-To: <CAKFQuwYg+HtK+zFKENbsrLgz9x8LL1HWau6DR7Om6o1dT=tt6g@mail.gmail.com>
References: <[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<CALtqXTeQ+QCFT0DWcM5LOjnf9_Fz59+ALFejNEFwpFhW-9o4ag@mail.gmail.com>
<CAKFQuwYg+HtK+zFKENbsrLgz9x8LL1HWau6DR7Om6o1dT=tt6g@mail.gmail.com>
On Thu, Jun 23, 2022 at 1:04 AM David G. Johnston <
[email protected]> wrote:
> On Wed, Jun 22, 2022 at 12:11 PM Ibrar Ahmed <[email protected]>
> wrote:
>
>> On Thu, Jun 23, 2022 at 12:01 AM Tom Lane <[email protected]> wrote:
>>
>>> Robert Haas <[email protected]> writes:
>>> > On Jun 2, 2009, at 9:41 AM, Simon Riggs <[email protected]> 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
Attachments:
[application/octet-stream] explain_float_row_v2.patch (3.4K, ../CALtqXTcrXtwZ3RRg2FH8yt8mDxRypXrUbw6a=zp0x2fKm_5xNg@mail.gmail.com/3-explain_float_row_v2.patch)
download | inline diff:
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index 47d80b0d25..bd60b55574 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -122,7 +122,7 @@ bool bsysscan = false;
* lookups as fast as possible.
*/
static FullTransactionId XactTopFullTransactionId = {InvalidTransactionId};
-static int nParallelCurrentXids = 0;
+static int nParallelCurrentXids = 0;
static TransactionId *ParallelCurrentXids;
/*
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index 5d1f7089da..d53c9c7235 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -1624,13 +1624,20 @@ ExplainNode(PlanState *planstate, List *ancestors,
if (es->format == EXPLAIN_FORMAT_TEXT)
{
if (es->timing)
+ {
+ appendStringInfo(es->str, " (actual time=%.3f..%.3f",
+ startup_ms, total_ms);
appendStringInfo(es->str,
- " (actual time=%.3f..%.3f rows=%.0f loops=%.0f)",
- startup_ms, total_ms, rows, nloops);
+ nloops == 1 ? " rows=%.0f loops=%.0f)" : " rows=%.2f loops=%.0f)",
+ rows, nloops);
+ }
else
+ {
appendStringInfo(es->str,
- " (actual rows=%.0f loops=%.0f)",
+ nloops == 1 ?
+ " (actual rows=%.0f loops=%.0f)" : " (actual rows=%.2f loops=%.0f)",
rows, nloops);
+ }
}
else
{
@@ -1641,7 +1648,7 @@ ExplainNode(PlanState *planstate, List *ancestors,
ExplainPropertyFloat("Actual Total Time", "ms", total_ms,
3, es);
}
- ExplainPropertyFloat("Actual Rows", NULL, rows, 0, es);
+ ExplainPropertyFloat("Actual Rows", NULL, rows, nloops == 1 ? 0 : 2, es);
ExplainPropertyFloat("Actual Loops", NULL, nloops, 0, es);
}
}
@@ -1690,13 +1697,19 @@ ExplainNode(PlanState *planstate, List *ancestors,
{
ExplainIndentText(es);
if (es->timing)
+ {
+ appendStringInfo(es->str, "actual time=%.3f..%.3f",
+ startup_ms, total_ms);
appendStringInfo(es->str,
- "actual time=%.3f..%.3f rows=%.0f loops=%.0f\n",
- startup_ms, total_ms, rows, nloops);
+ nloops == 1 ? " rows=%.0f loops=%.0f" : " rows=%.2f loops=%.0f",
+ rows, nloops);
+ }
else
- appendStringInfo(es->str,
- "actual rows=%.0f loops=%.0f\n",
+ {
+ appendStringInfo(es->str, nloops == 1 ?
+ "actual rows=%.0f loops=%.0f" : "actual rows=%.2f loops=%.0f",
rows, nloops);
+ }
}
else
{
@@ -1707,7 +1720,7 @@ ExplainNode(PlanState *planstate, List *ancestors,
ExplainPropertyFloat("Actual Total Time", "ms",
total_ms, 3, es);
}
- ExplainPropertyFloat("Actual Rows", NULL, rows, 0, es);
+ ExplainPropertyFloat("Actual Rows", NULL, rows, nloops == 1 ? 0 : 2, es);
ExplainPropertyFloat("Actual Loops", NULL, nloops, 0, es);
}
diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index ce12915592..baecc40e0c 100644
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -1937,7 +1937,7 @@ _outPathInfo(StringInfo str, const Path *node)
WRITE_BOOL_FIELD(parallel_aware);
WRITE_BOOL_FIELD(parallel_safe);
WRITE_INT_FIELD(parallel_workers);
- WRITE_FLOAT_FIELD(rows, "%.0f");
+ WRITE_FLOAT_FIELD(rows, "%.2f");
WRITE_FLOAT_FIELD(startup_cost, "%.2f");
WRITE_FLOAT_FIELD(total_cost, "%.2f");
WRITE_NODE_FIELD(pathkeys);
view thread (45+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: explain analyze rows=%.0f
In-Reply-To: <CALtqXTcrXtwZ3RRg2FH8yt8mDxRypXrUbw6a=zp0x2fKm_5xNg@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox