public inbox for [email protected]
help / color / mirror / Atom feedFrom: Ibrar Ahmed <[email protected]>
To: Amit Kapila <[email protected]>
Cc: 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: Fri, 8 Jul 2022 23:09:28 +0500
Message-ID: <CALtqXTdVDc=GUHQzd2v38J_nhm1tsiZobKmmyotmVij7TSyhkQ@mail.gmail.com> (raw)
In-Reply-To: <CAA4eK1K78Jg50wCbGOdD_gBHVSwdFdXmKUxV8uN4RRn9mz=r_Q@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>
<CALtqXTcrXtwZ3RRg2FH8yt8mDxRypXrUbw6a=zp0x2fKm_5xNg@mail.gmail.com>
<CAA4eK1K78Jg50wCbGOdD_gBHVSwdFdXmKUxV8uN4RRn9mz=r_Q@mail.gmail.com>
On Thu, Jul 7, 2022 at 2:41 PM Amit Kapila <[email protected]> wrote:
> On Thu, Jun 23, 2022 at 2:25 AM Ibrar Ahmed <[email protected]> wrote:
> >
> > On Thu, Jun 23, 2022 at 1:04 AM David G. Johnston <
> [email protected]> 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
Attachments:
[application/octet-stream] explain_float_row_v3.patch (6.2K, ../CALtqXTdVDc=GUHQzd2v38J_nhm1tsiZobKmmyotmVij7TSyhkQ@mail.gmail.com/3-explain_float_row_v3.patch)
download | inline diff:
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index e29c2ae206..ae078d86de 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -13,6 +13,8 @@
*/
#include "postgres.h"
+#include <math.h>
+
#include "access/xact.h"
#include "catalog/pg_type.h"
#include "commands/createas.h"
@@ -54,6 +56,9 @@ explain_get_index_name_hook_type explain_get_index_name_hook = NULL;
#define X_CLOSE_IMMEDIATE 2
#define X_NOWHITESPACE 4
+/* Check if float/double has any decimal number */
+#define HAS_DECIMAL(x) (floor(x) != x)
+
static void ExplainOneQuery(Query *query, int cursorOptions,
IntoClause *into, ExplainState *es,
const char *queryString, ParamListInfo params,
@@ -1643,16 +1648,29 @@ ExplainNode(PlanState *planstate, List *ancestors,
double total_ms = 1000.0 * planstate->instrument->total / nloops;
double rows = planstate->instrument->ntuples / nloops;
+ /*
+ * If the number of loops is greater than one, display the
+ * actual rows up to two decimal places instead of rounding
+ * off the value.
+ */
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 || !HAS_DECIMAL(rows)) ?
+ " rows=%.0f loops=%.0f)" : " rows=%.2f loops=%.0f)",
+ rows, nloops);
+ }
else
+ {
appendStringInfo(es->str,
- " (actual rows=%.0f loops=%.0f)",
+ (nloops == 1 || !HAS_DECIMAL(rows)) ?
+ " (actual rows=%.0f loops=%.0f)" : " (actual rows=%.2f loops=%.0f)",
rows, nloops);
+ }
}
else
{
@@ -1663,7 +1681,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 || !HAS_DECIMAL(rows)) ? 0 : 2, es);
ExplainPropertyFloat("Actual Loops", NULL, nloops, 0, es);
}
}
@@ -1707,18 +1725,30 @@ ExplainNode(PlanState *planstate, List *ancestors,
rows = instrument->ntuples / nloops;
ExplainOpenWorker(n, es);
+ /*
+ * If the number of loops is greater than one, display the
+ * actual rows up to two decimal places instead of rounding
+ * off the value.
+ */
if (es->format == EXPLAIN_FORMAT_TEXT)
{
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 || !HAS_DECIMAL(rows)) ?
+ " 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 || !HAS_DECIMAL(rows)) ?
+ "actual rows=%.0f loops=%.0f" : "actual rows=%.2f loops=%.0f",
rows, nloops);
+ }
}
else
{
@@ -1729,7 +1759,8 @@ 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 || !HAS_DECIMAL(rows)) ? 0 : 2, es);
ExplainPropertyFloat("Actual Loops", NULL, nloops, 0, es);
}
diff --git a/src/test/regress/expected/partition_prune.out b/src/test/regress/expected/partition_prune.out
index 7555764c77..002e5cc33d 100644
--- a/src/test/regress/expected/partition_prune.out
+++ b/src/test/regress/expected/partition_prune.out
@@ -2635,14 +2635,14 @@ order by tbl1.col1, tprt.col1;
insert into tbl1 values (1001), (1010), (1011);
explain (analyze, costs off, summary off, timing off)
select * from tbl1 inner join tprt on tbl1.col1 > tprt.col1;
- QUERY PLAN
---------------------------------------------------------------------------
+ QUERY PLAN
+-----------------------------------------------------------------------------
Nested Loop (actual rows=23 loops=1)
-> Seq Scan on tbl1 (actual rows=5 loops=1)
- -> Append (actual rows=5 loops=5)
+ -> Append (actual rows=4.60 loops=5)
-> Index Scan using tprt1_idx on tprt_1 (actual rows=2 loops=5)
Index Cond: (col1 < tbl1.col1)
- -> Index Scan using tprt2_idx on tprt_2 (actual rows=3 loops=4)
+ -> Index Scan using tprt2_idx on tprt_2 (actual rows=2.75 loops=4)
Index Cond: (col1 < tbl1.col1)
-> Index Scan using tprt3_idx on tprt_3 (actual rows=1 loops=2)
Index Cond: (col1 < tbl1.col1)
@@ -2656,16 +2656,16 @@ select * from tbl1 inner join tprt on tbl1.col1 > tprt.col1;
explain (analyze, costs off, summary off, timing off)
select * from tbl1 inner join tprt on tbl1.col1 = tprt.col1;
- QUERY PLAN
---------------------------------------------------------------------------
+ QUERY PLAN
+-----------------------------------------------------------------------------
Nested Loop (actual rows=3 loops=1)
-> Seq Scan on tbl1 (actual rows=5 loops=1)
- -> Append (actual rows=1 loops=5)
+ -> Append (actual rows=0.60 loops=5)
-> Index Scan using tprt1_idx on tprt_1 (never executed)
Index Cond: (col1 = tbl1.col1)
-> Index Scan using tprt2_idx on tprt_2 (actual rows=1 loops=2)
Index Cond: (col1 = tbl1.col1)
- -> Index Scan using tprt3_idx on tprt_3 (actual rows=0 loops=3)
+ -> Index Scan using tprt3_idx on tprt_3 (actual rows=0.33 loops=3)
Index Cond: (col1 = tbl1.col1)
-> Index Scan using tprt4_idx on tprt_4 (never executed)
Index Cond: (col1 = tbl1.col1)
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], [email protected]
Subject: Re: explain analyze rows=%.0f
In-Reply-To: <CALtqXTdVDc=GUHQzd2v38J_nhm1tsiZobKmmyotmVij7TSyhkQ@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