public inbox for [email protected]  
help / color / mirror / Atom feed
[PATCH v7 11/16] Remove heap_freeze_execute_prepared()
26+ messages / 7 participants
[nested] [flat]

* [PATCH v7 11/16] Remove heap_freeze_execute_prepared()
@ 2024-03-26 13:10 Melanie Plageman <[email protected]>
  0 siblings, 0 replies; 26+ messages in thread

From: Melanie Plageman @ 2024-03-26 13:10 UTC (permalink / raw)

In order to merge freeze and prune records, the execution of tuple
freezing and the WAL logging of the changes to the page must be
separated so that the WAL logging can be combined with prune WAL
logging. This commit makes a helper for the tuple freezing and then
inlines the contents of heap_freeze_execute_prepared() where it is
called in heap_page_prune().
---
 src/backend/access/heap/heapam.c    | 49 +++++++----------------------
 src/backend/access/heap/pruneheap.c | 22 ++++++++++---
 src/include/access/heapam.h         | 28 +++++++++--------
 3 files changed, 44 insertions(+), 55 deletions(-)

diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index be48098f7f3..1c1785994b1 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -6340,9 +6340,9 @@ FreezeMultiXactId(MultiXactId multi, uint16 t_infomask,
  * XIDs or MultiXactIds that will need to be processed by a future VACUUM.
  *
  * VACUUM caller must assemble HeapTupleFreeze freeze plan entries for every
- * tuple that we returned true for, and call heap_freeze_execute_prepared to
- * execute freezing.  Caller must initialize pagefrz fields for page as a
- * whole before first call here for each heap page.
+ * tuple that we returned true for, and then execute freezing.  Caller must
+ * initialize pagefrz fields for page as a whole before first call here for
+ * each heap page.
  *
  * VACUUM caller decides on whether or not to freeze the page as a whole.
  * We'll often prepare freeze plans for a page that caller just discards.
@@ -6657,8 +6657,8 @@ heap_execute_freeze_tuple(HeapTupleHeader tuple, HeapTupleFreeze *frz)
 }
 
 /*
-* Perform xmin/xmax XID status sanity checks before calling
-* heap_freeze_execute_prepared().
+* Perform xmin/xmax XID status sanity checks before actually executing freeze
+* plans.
 *
 * heap_prepare_freeze_tuple doesn't perform these checks directly because
 * pg_xact lookups are relatively expensive.  They shouldn't be repeated
@@ -6711,30 +6711,17 @@ heap_pre_freeze_checks(Buffer buffer,
 }
 
 /*
- * heap_freeze_execute_prepared
- *
- * Executes freezing of one or more heap tuples on a page on behalf of caller.
- * Caller passes an array of tuple plans from heap_prepare_freeze_tuple.
- * Caller must set 'offset' in each plan for us.  Note that we destructively
- * sort caller's tuples array in-place, so caller had better be done with it.
- *
- * WAL-logs the changes so that VACUUM can advance the rel's relfrozenxid
- * later on without any risk of unsafe pg_xact lookups, even following a hard
- * crash (or when querying from a standby).  We represent freezing by setting
- * infomask bits in tuple headers, but this shouldn't be thought of as a hint.
- * See section on buffer access rules in src/backend/storage/buffer/README.
+ * Helper which executes freezing of one or more heap tuples on a page on
+ * behalf of caller. Caller passes an array of tuple plans from
+ * heap_prepare_freeze_tuple. Caller must set 'offset' in each plan for us.
+ * Must be called in a critical section that also marks the buffer dirty and,
+ * if needed, emits WAL.
  */
 void
-heap_freeze_execute_prepared(Relation rel, Buffer buffer,
-							 TransactionId snapshotConflictHorizon,
-							 HeapTupleFreeze *tuples, int ntuples)
+heap_freeze_prepared_tuples(Buffer buffer, HeapTupleFreeze *tuples, int ntuples)
 {
 	Page		page = BufferGetPage(buffer);
 
-	Assert(ntuples > 0);
-
-	START_CRIT_SECTION();
-
 	for (int i = 0; i < ntuples; i++)
 	{
 		HeapTupleFreeze *frz = tuples + i;
@@ -6746,20 +6733,6 @@ heap_freeze_execute_prepared(Relation rel, Buffer buffer,
 	}
 
 	MarkBufferDirty(buffer);
-
-	/* Now WAL-log freezing if necessary */
-	if (RelationNeedsWAL(rel))
-	{
-		log_heap_prune_and_freeze(rel, buffer, snapshotConflictHorizon,
-								  false,	/* no cleanup lock required */
-								  PRUNE_VACUUM_SCAN,
-								  tuples, ntuples,
-								  NULL, 0,	/* redirected */
-								  NULL, 0,	/* dead */
-								  NULL, 0); /* unused */
-	}
-
-	END_CRIT_SECTION();
 }
 
 /*
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index fe463ad7146..8914d4bf5c8 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -635,10 +635,24 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
 
 	if (do_freeze)
 	{
-		/* Execute all freeze plans for page as a single atomic action */
-		heap_freeze_execute_prepared(relation, buffer,
-									 frz_conflict_horizon,
-									 prstate.frozen, presult->nfrozen);
+		START_CRIT_SECTION();
+
+		Assert(presult->nfrozen > 0);
+
+		heap_freeze_prepared_tuples(buffer, prstate.frozen, presult->nfrozen);
+
+		MarkBufferDirty(buffer);
+
+		/* Now WAL-log freezing if necessary */
+		if (RelationNeedsWAL(relation))
+			log_heap_prune_and_freeze(relation, buffer,
+									  frz_conflict_horizon, false, reason,
+									  prstate.frozen, presult->nfrozen,
+									  NULL, 0,	/* redirected */
+									  NULL, 0,	/* dead */
+									  NULL, 0); /* unused */
+
+		END_CRIT_SECTION();
 	}
 	else if (!pagefrz || !presult->all_frozen || presult->nfrozen > 0)
 	{
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index dbf6323b5ff..ac0ef6e4281 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -14,6 +14,7 @@
 #ifndef HEAPAM_H
 #define HEAPAM_H
 
+#include "access/heapam_xlog.h"
 #include "access/relation.h"	/* for backward compatibility */
 #include "access/relscan.h"
 #include "access/sdir.h"
@@ -101,8 +102,8 @@ typedef enum
 } HTSV_Result;
 
 /*
- * heap_prepare_freeze_tuple may request that heap_freeze_execute_prepared
- * check any tuple's to-be-frozen xmin and/or xmax status using pg_xact
+ * heap_prepare_freeze_tuple may request that any tuple's to-be-frozen xmin
+ * and/or xmax status is checked using pg_xact during freezing execution.
  */
 #define		HEAP_FREEZE_CHECK_XMIN_COMMITTED	0x01
 #define		HEAP_FREEZE_CHECK_XMAX_ABORTED		0x02
@@ -154,14 +155,14 @@ typedef struct HeapPageFreeze
 	/*
 	 * "Freeze" NewRelfrozenXid/NewRelminMxid trackers.
 	 *
-	 * Trackers used when heap_freeze_execute_prepared freezes, or when there
-	 * are zero freeze plans for a page.  It is always valid for vacuumlazy.c
-	 * to freeze any page, by definition.  This even includes pages that have
-	 * no tuples with storage to consider in the first place.  That way the
-	 * 'totally_frozen' results from heap_prepare_freeze_tuple can always be
-	 * used in the same way, even when no freeze plans need to be executed to
-	 * "freeze the page".  Only the "freeze" path needs to consider the need
-	 * to set pages all-frozen in the visibility map under this scheme.
+	 * Trackers used when tuples will be frozen, or when there are zero freeze
+	 * plans for a page.  It is always valid for vacuumlazy.c to freeze any
+	 * page, by definition.  This even includes pages that have no tuples with
+	 * storage to consider in the first place.  That way the 'totally_frozen'
+	 * results from heap_prepare_freeze_tuple can always be used in the same
+	 * way, even when no freeze plans need to be executed to "freeze the
+	 * page". Only the "freeze" path needs to consider the need to set pages
+	 * all-frozen in the visibility map under this scheme.
 	 *
 	 * When we freeze a page, we generally freeze all XIDs < OldestXmin, only
 	 * leaving behind XIDs that are ineligible for freezing, if any.  And so
@@ -343,12 +344,13 @@ extern bool heap_prepare_freeze_tuple(HeapTupleHeader tuple,
 
 extern void heap_pre_freeze_checks(Buffer buffer,
 								   HeapTupleFreeze *tuples, int ntuples);
-extern void heap_freeze_execute_prepared(Relation rel, Buffer buffer,
-										 TransactionId snapshotConflictHorizon,
-										 HeapTupleFreeze *tuples, int ntuples);
+
+extern void heap_freeze_prepared_tuples(Buffer buffer,
+										HeapTupleFreeze *tuples, int ntuples);
 extern bool heap_freeze_tuple(HeapTupleHeader tuple,
 							  TransactionId relfrozenxid, TransactionId relminmxid,
 							  TransactionId FreezeLimit, TransactionId MultiXactCutoff);
+
 extern bool heap_tuple_should_freeze(HeapTupleHeader tuple,
 									 const struct VacuumCutoffs *cutoffs,
 									 TransactionId *NoFreezePageRelfrozenXid,
-- 
2.40.1


--ck6erxojvlx23byk
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v7-0012-Merge-prune-and-freeze-records.patch"



^ permalink  raw  reply  [nested|flat] 26+ messages in thread

* Re: explain analyze rows=%.0f
@ 2025-01-13 20:18 Ilia Evdokimov <[email protected]>
  2025-02-07 19:59 ` Re: explain analyze rows=%.0f Matheus Alcantara <[email protected]>
  0 siblings, 1 reply; 26+ messages in thread

From: Ilia Evdokimov @ 2025-01-13 20:18 UTC (permalink / raw)
  To: Guillaume Lelarge <[email protected]>; +Cc: Daniel Gustafsson <[email protected]>; Ibrar Ahmed <[email protected]>; Gregory Stark (as CFM) <[email protected]>; Tom Lane <[email protected]>; Robert Haas <[email protected]>; Amit Kapila <[email protected]>; Justin Pryzby <[email protected]>; Peter Geoghegan <[email protected]>; vignesh C <[email protected]>; David G. Johnston <[email protected]>; [email protected]


On 11.01.2025 14:10, Ilia Evdokimov wrote:
>
>
> On 11.01.2025 12:15, Guillaume Lelarge wrote:
>>
>>
>> Thanks for your patch, this looks like a very interesting feature 
>> that I'd like to see in a future release.
>>
>> It did a quick run: patch OK, make OK, make install OK, but make 
>> check fails quite a lot on partition_prune.sql.
>>
>> I guess it would need some work on partition_prune.sql tests and 
>> perhaps also on the docs.
>>
>> Thanks again.
>>
>>
>> -- 
>> Guillaume.
>
>
> Yes, certainly. I have fixed partition_prune.sql. In the documentation 
> example for EXPLAIN ANALYZE where loops is greater than one, I updated 
> how 'rows' and 'loops' values are displayed so they appear as decimal 
> fractions with two digits after the decimal point.
>
> I attached fixed patch.
>
> Any suggestions?
>
> --
> Best regards,
> Ilia Evdokimov,
> Tantor Labs LLC.
>

I guess, it's not ideal to modify the existing example in the 
documentation of the v5 patch because readers wouldn't immediately 
understand why decimal fractions appear there. Instead, I'll add a brief 
note in the documentation clarifying how rows and loops are displayed 
when the average row count is below one.

The changes the of documentation are attached v6 patch.

If you have any other suggestions or different opinions, I'd be happy to 
discuss them.

--
Best regards,
Ilia Evdokimov,
Tantor Labs LLC.


Attachments:

  [text/x-patch] v6-0001-Clarify-display-of-rows-and-loops-as-decimal-fraction.patch (7.2K, ../../[email protected]/3-v6-0001-Clarify-display-of-rows-and-loops-as-decimal-fraction.patch)
  download | inline diff:
From 08f92c7e11829045014598e1dcc042f3e5a1e1a3 Mon Sep 17 00:00:00 2001
From: Ilia Evdokimov <[email protected]>
Date: Mon, 13 Jan 2025 23:01:44 +0300
Subject: [PATCH] Clarify display of rows and loops as decimal fractions

When the average number of rows is small compared to the number of loops,
both rows and loops are displayed as decimal fractions with two digits
after the decimal point in EXPLAIN ANALYZE.
---
 doc/src/sgml/perform.sgml                     |  6 ++-
 src/backend/commands/explain.c                | 49 +++++++++++++------
 src/test/regress/expected/partition_prune.out | 10 ++--
 src/test/regress/sql/partition_prune.sql      |  2 +-
 4 files changed, 44 insertions(+), 23 deletions(-)

diff --git a/doc/src/sgml/perform.sgml b/doc/src/sgml/perform.sgml
index a502a2aaba..3f13d17fe9 100644
--- a/doc/src/sgml/perform.sgml
+++ b/doc/src/sgml/perform.sgml
@@ -757,7 +757,11 @@ WHERE t1.unique1 &lt; 10 AND t1.unique2 = t2.unique2;
     comparable with the way that the cost estimates are shown.  Multiply by
     the <literal>loops</literal> value to get the total time actually spent in
     the node.  In the above example, we spent a total of 0.030 milliseconds
-    executing the index scans on <literal>tenk2</literal>.
+    executing the index scans on <literal>tenk2</literal>.   If a subplan node
+    is executed multiple times and the average number of rows is less than one,
+    the rows and <literal>loops</literal> values are shown as a decimal fraction
+    (with two digits after the decimal point) to indicate that some rows
+    were actually processed rather than simply rounding down to zero.
    </para>
 
    <para>
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index c24e66f82e..200294b756 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -1981,14 +1981,14 @@ ExplainNode(PlanState *planstate, List *ancestors,
 
 		if (es->format == EXPLAIN_FORMAT_TEXT)
 		{
+			appendStringInfo(es->str, " (actual");
 			if (es->timing)
-				appendStringInfo(es->str,
-								 " (actual time=%.3f..%.3f rows=%.0f loops=%.0f)",
-								 startup_ms, total_ms, rows, nloops);
+				appendStringInfo(es->str, " time=%.3f..%.3f", startup_ms, total_ms);
+
+			if (nloops > 1 && planstate->instrument->ntuples < nloops)
+				appendStringInfo(es->str," rows=%.2f loops=%.2f)", rows, nloops);
 			else
-				appendStringInfo(es->str,
-								 " (actual rows=%.0f loops=%.0f)",
-								 rows, nloops);
+				appendStringInfo(es->str," rows=%.0f loops=%.0f)", rows, nloops);
 		}
 		else
 		{
@@ -1999,8 +1999,16 @@ ExplainNode(PlanState *planstate, List *ancestors,
 				ExplainPropertyFloat("Actual Total Time", "ms", total_ms,
 									 3, es);
 			}
-			ExplainPropertyFloat("Actual Rows", NULL, rows, 0, es);
-			ExplainPropertyFloat("Actual Loops", NULL, nloops, 0, es);
+			if (nloops > 1 && planstate->instrument->ntuples < nloops)
+			{
+				ExplainPropertyFloat("Actual Rows", NULL, rows, 2, es);
+				ExplainPropertyFloat("Actual Loops", NULL, nloops, 2, es);
+			}
+			else
+			{
+				ExplainPropertyFloat("Actual Rows", NULL, rows, 0, es);
+				ExplainPropertyFloat("Actual Loops", NULL, nloops, 0, es);
+			}
 		}
 	}
 	else if (es->analyze)
@@ -2052,14 +2060,14 @@ ExplainNode(PlanState *planstate, List *ancestors,
 			if (es->format == EXPLAIN_FORMAT_TEXT)
 			{
 				ExplainIndentText(es);
+				appendStringInfo(es->str, "actual");
 				if (es->timing)
-					appendStringInfo(es->str,
-									 "actual time=%.3f..%.3f rows=%.0f loops=%.0f\n",
-									 startup_ms, total_ms, rows, nloops);
+					appendStringInfo(es->str, " time=%.3f..%.3f", startup_ms, total_ms);
+
+				if (nloops > 1 && planstate->instrument->ntuples < nloops)
+					appendStringInfo(es->str," rows=%.2f loops=%.2f)", rows, nloops);
 				else
-					appendStringInfo(es->str,
-									 "actual rows=%.0f loops=%.0f\n",
-									 rows, nloops);
+					appendStringInfo(es->str," rows=%.0f loops=%.0f)", rows, nloops);
 			}
 			else
 			{
@@ -2070,8 +2078,17 @@ ExplainNode(PlanState *planstate, List *ancestors,
 					ExplainPropertyFloat("Actual Total Time", "ms",
 										 total_ms, 3, es);
 				}
-				ExplainPropertyFloat("Actual Rows", NULL, rows, 0, es);
-				ExplainPropertyFloat("Actual Loops", NULL, nloops, 0, es);
+
+				if (nloops > 1 && planstate->instrument->ntuples < nloops)
+				{
+					ExplainPropertyFloat("Actual Rows", NULL, rows, 2, es);
+					ExplainPropertyFloat("Actual Loops", NULL, nloops, 2, es);
+				}
+				else
+				{
+					ExplainPropertyFloat("Actual Rows", NULL, rows, 0, es);
+					ExplainPropertyFloat("Actual Loops", NULL, nloops, 0, es);
+				}
 			}
 
 			ExplainCloseWorker(n, es);
diff --git a/src/test/regress/expected/partition_prune.out b/src/test/regress/expected/partition_prune.out
index c52bc40e81..dc6a3cb54a 100644
--- a/src/test/regress/expected/partition_prune.out
+++ b/src/test/regress/expected/partition_prune.out
@@ -2338,7 +2338,7 @@ begin
             $1)
     loop
         ln := regexp_replace(ln, 'Workers Launched: \d+', 'Workers Launched: N');
-        ln := regexp_replace(ln, 'actual rows=\d+ loops=\d+', 'actual rows=N loops=N');
+        ln := regexp_replace(ln, 'actual rows=\d+(?:\.\d+)? loops=\d+(?:\.\d+)?', 'actual rows=N loops=N');
         ln := regexp_replace(ln, 'Rows Removed by Filter: \d+', 'Rows Removed by Filter: N');
         return next ln;
     end loop;
@@ -3041,16 +3041,16 @@ select * from tbl1 inner join tprt on tbl1.col1 > tprt.col1;
 
 explain (analyze, costs off, summary off, timing off, buffers 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.00)
          ->  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.00)
                Index Cond: (col1 = tbl1.col1)
          ->  Index Scan using tprt4_idx on tprt_4 (never executed)
                Index Cond: (col1 = tbl1.col1)
diff --git a/src/test/regress/sql/partition_prune.sql b/src/test/regress/sql/partition_prune.sql
index d67598d5c7..bb1e09a536 100644
--- a/src/test/regress/sql/partition_prune.sql
+++ b/src/test/regress/sql/partition_prune.sql
@@ -571,7 +571,7 @@ begin
             $1)
     loop
         ln := regexp_replace(ln, 'Workers Launched: \d+', 'Workers Launched: N');
-        ln := regexp_replace(ln, 'actual rows=\d+ loops=\d+', 'actual rows=N loops=N');
+        ln := regexp_replace(ln, 'actual rows=\d+(?:\.\d+)? loops=\d+(?:\.\d+)?', 'actual rows=N loops=N');
         ln := regexp_replace(ln, 'Rows Removed by Filter: \d+', 'Rows Removed by Filter: N');
         return next ln;
     end loop;
-- 
2.34.1



^ permalink  raw  reply  [nested|flat] 26+ messages in thread

* Re: explain analyze rows=%.0f
  2025-01-13 20:18 Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
@ 2025-02-07 19:59 ` Matheus Alcantara <[email protected]>
  2025-02-07 20:41   ` Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
  0 siblings, 1 reply; 26+ messages in thread

From: Matheus Alcantara @ 2025-02-07 19:59 UTC (permalink / raw)
  To: Ilia Evdokimov <[email protected]>; +Cc: Guillaume Lelarge <[email protected]>; Daniel Gustafsson <[email protected]>; Ibrar Ahmed <[email protected]>; Gregory Stark (as CFM) <[email protected]>; Tom Lane <[email protected]>; Robert Haas <[email protected]>; Amit Kapila <[email protected]>; Justin Pryzby <[email protected]>; Peter Geoghegan <[email protected]>; vignesh C <[email protected]>; David G. Johnston <[email protected]>; [email protected]

Hi

Em seg., 13 de jan. de 2025 às 17:18, Ilia Evdokimov
<[email protected]> escreveu:
> I guess, it's not ideal to modify the existing example in the documentation of the v5 patch because readers wouldn't immediately understand why decimal fractions appear there. Instead, I'll add a brief note in the documentation clarifying how rows and loops are displayed when the average row count is below one.
>
> The changes the of documentation are attached v6 patch.
>
v6 is not applying on master, could you please rebase?

git apply v6-0001-Clarify-display-of-rows-and-loops-as-decimal-fraction.patch
error: patch failed: src/test/regress/expected/partition_prune.out:3041
error: src/test/regress/expected/partition_prune.out: patch does not apply

-- 
Matheus Alcantara






^ permalink  raw  reply  [nested|flat] 26+ messages in thread

* Re: explain analyze rows=%.0f
  2025-01-13 20:18 Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
  2025-02-07 19:59 ` Re: explain analyze rows=%.0f Matheus Alcantara <[email protected]>
@ 2025-02-07 20:41   ` Ilia Evdokimov <[email protected]>
  2025-02-07 21:01     ` Re: explain analyze rows=%.0f Matheus Alcantara <[email protected]>
  0 siblings, 1 reply; 26+ messages in thread

From: Ilia Evdokimov @ 2025-02-07 20:41 UTC (permalink / raw)
  To: Matheus Alcantara <[email protected]>; +Cc: Guillaume Lelarge <[email protected]>; Daniel Gustafsson <[email protected]>; Ibrar Ahmed <[email protected]>; Gregory Stark (as CFM) <[email protected]>; Tom Lane <[email protected]>; Robert Haas <[email protected]>; Amit Kapila <[email protected]>; Justin Pryzby <[email protected]>; Peter Geoghegan <[email protected]>; vignesh C <[email protected]>; David G. Johnston <[email protected]>; [email protected]


On 07.02.2025 22:59, Matheus Alcantara wrote:
> Hi
> v6 is not applying on master, could you please rebase?
>
> git apply v6-0001-Clarify-display-of-rows-and-loops-as-decimal-fraction.patch
> error: patch failed: src/test/regress/expected/partition_prune.out:3041
> error: src/test/regress/expected/partition_prune.out: patch does not apply
>
Hi

Strange, I don't have any problems to apply it on master.

postgres$ git branch
     * master
postgres$ git pull
     Already up to date.
postgres$ git apply 
v6-0001-Clarify-display-of-rows-and-loops-as-decimal-fraction.patch
postgres$

--
Best regards,
Ilia Evdokimov,
Tantor Labs LLC.








^ permalink  raw  reply  [nested|flat] 26+ messages in thread

* Re: explain analyze rows=%.0f
  2025-01-13 20:18 Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
  2025-02-07 19:59 ` Re: explain analyze rows=%.0f Matheus Alcantara <[email protected]>
  2025-02-07 20:41   ` Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
@ 2025-02-07 21:01     ` Matheus Alcantara <[email protected]>
  2025-02-07 21:28       ` Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
  0 siblings, 1 reply; 26+ messages in thread

From: Matheus Alcantara @ 2025-02-07 21:01 UTC (permalink / raw)
  To: Ilia Evdokimov <[email protected]>; +Cc: Guillaume Lelarge <[email protected]>; Daniel Gustafsson <[email protected]>; Ibrar Ahmed <[email protected]>; Gregory Stark (as CFM) <[email protected]>; Tom Lane <[email protected]>; Robert Haas <[email protected]>; Amit Kapila <[email protected]>; Justin Pryzby <[email protected]>; Peter Geoghegan <[email protected]>; vignesh C <[email protected]>; David G. Johnston <[email protected]>; [email protected]

Em sex., 7 de fev. de 2025 às 17:41, Ilia Evdokimov
<[email protected]> escreveu:
> Strange, I don't have any problems to apply it on master.
>
> postgres$ git branch
>      * master
> postgres$ git pull
>      Already up to date.
> postgres$ git apply
> v6-0001-Clarify-display-of-rows-and-loops-as-decimal-fraction.patch
> postgres$

Just for reference I'm trying to apply based on commit fb056564ec5.
https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=fb056564ec5bc1c18dd670c963c893cdb6de9...

-- 
Matheus Alcantara






^ permalink  raw  reply  [nested|flat] 26+ messages in thread

* Re: explain analyze rows=%.0f
  2025-01-13 20:18 Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
  2025-02-07 19:59 ` Re: explain analyze rows=%.0f Matheus Alcantara <[email protected]>
  2025-02-07 20:41   ` Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
  2025-02-07 21:01     ` Re: explain analyze rows=%.0f Matheus Alcantara <[email protected]>
@ 2025-02-07 21:28       ` Ilia Evdokimov <[email protected]>
  2025-02-10 15:32         ` Re: explain analyze rows=%.0f Matheus Alcantara <[email protected]>
  2025-02-11 17:14         ` Re: explain analyze rows=%.0f Andrei Lepikhov <[email protected]>
  0 siblings, 2 replies; 26+ messages in thread

From: Ilia Evdokimov @ 2025-02-07 21:28 UTC (permalink / raw)
  To: Matheus Alcantara <[email protected]>; +Cc: Guillaume Lelarge <[email protected]>; Daniel Gustafsson <[email protected]>; Ibrar Ahmed <[email protected]>; Gregory Stark (as CFM) <[email protected]>; Tom Lane <[email protected]>; Robert Haas <[email protected]>; Amit Kapila <[email protected]>; Justin Pryzby <[email protected]>; Peter Geoghegan <[email protected]>; vignesh C <[email protected]>; David G. Johnston <[email protected]>; [email protected]


On 08.02.2025 00:01, Matheus Alcantara wrote:
> Em sex., 7 de fev. de 2025 às 17:41, Ilia Evdokimov
> <[email protected]> escreveu:
>> Strange, I don't have any problems to apply it on master.
>>
>> postgres$ git branch
>>       * master
>> postgres$ git pull
>>       Already up to date.
>> postgres$ git apply
>> v6-0001-Clarify-display-of-rows-and-loops-as-decimal-fraction.patch
>> postgres$
> Just for reference I'm trying to apply based on commit fb056564ec5.
> https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=fb056564ec5bc1c18dd670c963c893cdb6de9...
>

You are right, because two commits were appeared after creating v6-patch 
on partition_prune.out and patch v6 must not have applied on master. 
Then I created v7 patch rebased on fb056564ec5 . Thank for your remark!

--
Best regards,
Ilia Evdokimov,
Tantor Labs LLC.


Attachments:

  [text/x-patch] v7-0001-Clarify-display-of-rows-and-loops-as-decimal-fraction.patch (7.2K, ../../[email protected]/2-v7-0001-Clarify-display-of-rows-and-loops-as-decimal-fraction.patch)
  download | inline diff:
From d94d30618b6f363a54afe5457ef8fe89476e096e Mon Sep 17 00:00:00 2001
From: Ilia Evdokimov <[email protected]>
Date: Sat, 8 Feb 2025 00:15:54 +0300
Subject: [PATCH] Clarify display of rows and loops as decimal fractions

When the average number of rows is small compared to the number of loops,
both rows and loops are displayed as decimal fractions with two digits
after the decimal point in EXPLAIN ANALYZE.
---
 doc/src/sgml/perform.sgml                     |  6 ++-
 src/backend/commands/explain.c                | 49 +++++++++++++------
 src/test/regress/expected/partition_prune.out | 10 ++--
 src/test/regress/sql/partition_prune.sql      |  2 +-
 4 files changed, 44 insertions(+), 23 deletions(-)

diff --git a/doc/src/sgml/perform.sgml b/doc/src/sgml/perform.sgml
index a502a2aaba..3f13d17fe9 100644
--- a/doc/src/sgml/perform.sgml
+++ b/doc/src/sgml/perform.sgml
@@ -757,7 +757,11 @@ WHERE t1.unique1 &lt; 10 AND t1.unique2 = t2.unique2;
     comparable with the way that the cost estimates are shown.  Multiply by
     the <literal>loops</literal> value to get the total time actually spent in
     the node.  In the above example, we spent a total of 0.030 milliseconds
-    executing the index scans on <literal>tenk2</literal>.
+    executing the index scans on <literal>tenk2</literal>.   If a subplan node
+    is executed multiple times and the average number of rows is less than one,
+    the rows and <literal>loops</literal> values are shown as a decimal fraction
+    (with two digits after the decimal point) to indicate that some rows
+    were actually processed rather than simply rounding down to zero.
    </para>
 
    <para>
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index c24e66f82e..200294b756 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -1981,14 +1981,14 @@ ExplainNode(PlanState *planstate, List *ancestors,
 
 		if (es->format == EXPLAIN_FORMAT_TEXT)
 		{
+			appendStringInfo(es->str, " (actual");
 			if (es->timing)
-				appendStringInfo(es->str,
-								 " (actual time=%.3f..%.3f rows=%.0f loops=%.0f)",
-								 startup_ms, total_ms, rows, nloops);
+				appendStringInfo(es->str, " time=%.3f..%.3f", startup_ms, total_ms);
+
+			if (nloops > 1 && planstate->instrument->ntuples < nloops)
+				appendStringInfo(es->str," rows=%.2f loops=%.2f)", rows, nloops);
 			else
-				appendStringInfo(es->str,
-								 " (actual rows=%.0f loops=%.0f)",
-								 rows, nloops);
+				appendStringInfo(es->str," rows=%.0f loops=%.0f)", rows, nloops);
 		}
 		else
 		{
@@ -1999,8 +1999,16 @@ ExplainNode(PlanState *planstate, List *ancestors,
 				ExplainPropertyFloat("Actual Total Time", "ms", total_ms,
 									 3, es);
 			}
-			ExplainPropertyFloat("Actual Rows", NULL, rows, 0, es);
-			ExplainPropertyFloat("Actual Loops", NULL, nloops, 0, es);
+			if (nloops > 1 && planstate->instrument->ntuples < nloops)
+			{
+				ExplainPropertyFloat("Actual Rows", NULL, rows, 2, es);
+				ExplainPropertyFloat("Actual Loops", NULL, nloops, 2, es);
+			}
+			else
+			{
+				ExplainPropertyFloat("Actual Rows", NULL, rows, 0, es);
+				ExplainPropertyFloat("Actual Loops", NULL, nloops, 0, es);
+			}
 		}
 	}
 	else if (es->analyze)
@@ -2052,14 +2060,14 @@ ExplainNode(PlanState *planstate, List *ancestors,
 			if (es->format == EXPLAIN_FORMAT_TEXT)
 			{
 				ExplainIndentText(es);
+				appendStringInfo(es->str, "actual");
 				if (es->timing)
-					appendStringInfo(es->str,
-									 "actual time=%.3f..%.3f rows=%.0f loops=%.0f\n",
-									 startup_ms, total_ms, rows, nloops);
+					appendStringInfo(es->str, " time=%.3f..%.3f", startup_ms, total_ms);
+
+				if (nloops > 1 && planstate->instrument->ntuples < nloops)
+					appendStringInfo(es->str," rows=%.2f loops=%.2f)", rows, nloops);
 				else
-					appendStringInfo(es->str,
-									 "actual rows=%.0f loops=%.0f\n",
-									 rows, nloops);
+					appendStringInfo(es->str," rows=%.0f loops=%.0f)", rows, nloops);
 			}
 			else
 			{
@@ -2070,8 +2078,17 @@ ExplainNode(PlanState *planstate, List *ancestors,
 					ExplainPropertyFloat("Actual Total Time", "ms",
 										 total_ms, 3, es);
 				}
-				ExplainPropertyFloat("Actual Rows", NULL, rows, 0, es);
-				ExplainPropertyFloat("Actual Loops", NULL, nloops, 0, es);
+
+				if (nloops > 1 && planstate->instrument->ntuples < nloops)
+				{
+					ExplainPropertyFloat("Actual Rows", NULL, rows, 2, es);
+					ExplainPropertyFloat("Actual Loops", NULL, nloops, 2, es);
+				}
+				else
+				{
+					ExplainPropertyFloat("Actual Rows", NULL, rows, 0, es);
+					ExplainPropertyFloat("Actual Loops", NULL, nloops, 0, es);
+				}
 			}
 
 			ExplainCloseWorker(n, es);
diff --git a/src/test/regress/expected/partition_prune.out b/src/test/regress/expected/partition_prune.out
index e667503c96..b45bb83c0f 100644
--- a/src/test/regress/expected/partition_prune.out
+++ b/src/test/regress/expected/partition_prune.out
@@ -2367,7 +2367,7 @@ begin
             $1)
     loop
         ln := regexp_replace(ln, 'Workers Launched: \d+', 'Workers Launched: N');
-        ln := regexp_replace(ln, 'actual rows=\d+ loops=\d+', 'actual rows=N loops=N');
+        ln := regexp_replace(ln, 'actual rows=\d+(?:\.\d+)? loops=\d+(?:\.\d+)?', 'actual rows=N loops=N');
         ln := regexp_replace(ln, 'Rows Removed by Filter: \d+', 'Rows Removed by Filter: N');
         return next ln;
     end loop;
@@ -3070,16 +3070,16 @@ select * from tbl1 inner join tprt on tbl1.col1 > tprt.col1;
 
 explain (analyze, costs off, summary off, timing off, buffers 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.00)
          ->  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.00)
                Index Cond: (col1 = tbl1.col1)
          ->  Index Scan using tprt4_idx on tprt_4 (never executed)
                Index Cond: (col1 = tbl1.col1)
diff --git a/src/test/regress/sql/partition_prune.sql b/src/test/regress/sql/partition_prune.sql
index 730545e86a..ea38ae6990 100644
--- a/src/test/regress/sql/partition_prune.sql
+++ b/src/test/regress/sql/partition_prune.sql
@@ -586,7 +586,7 @@ begin
             $1)
     loop
         ln := regexp_replace(ln, 'Workers Launched: \d+', 'Workers Launched: N');
-        ln := regexp_replace(ln, 'actual rows=\d+ loops=\d+', 'actual rows=N loops=N');
+        ln := regexp_replace(ln, 'actual rows=\d+(?:\.\d+)? loops=\d+(?:\.\d+)?', 'actual rows=N loops=N');
         ln := regexp_replace(ln, 'Rows Removed by Filter: \d+', 'Rows Removed by Filter: N');
         return next ln;
     end loop;
-- 
2.34.1



^ permalink  raw  reply  [nested|flat] 26+ messages in thread

* Re: explain analyze rows=%.0f
  2025-01-13 20:18 Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
  2025-02-07 19:59 ` Re: explain analyze rows=%.0f Matheus Alcantara <[email protected]>
  2025-02-07 20:41   ` Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
  2025-02-07 21:01     ` Re: explain analyze rows=%.0f Matheus Alcantara <[email protected]>
  2025-02-07 21:28       ` Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
@ 2025-02-10 15:32         ` Matheus Alcantara <[email protected]>
  2025-02-10 16:38           ` Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
  1 sibling, 1 reply; 26+ messages in thread

From: Matheus Alcantara @ 2025-02-10 15:32 UTC (permalink / raw)
  To: Ilia Evdokimov <[email protected]>; +Cc: Guillaume Lelarge <[email protected]>; Daniel Gustafsson <[email protected]>; Ibrar Ahmed <[email protected]>; Gregory Stark (as CFM) <[email protected]>; Tom Lane <[email protected]>; Robert Haas <[email protected]>; Amit Kapila <[email protected]>; Justin Pryzby <[email protected]>; Peter Geoghegan <[email protected]>; vignesh C <[email protected]>; David G. Johnston <[email protected]>; [email protected]

Thanks for the new patch version.

-- v7-0001-Clarify-display-of-rows-and-loops-as-decimal-fraction.patch
> +                if (nloops > 1 && planstate->instrument->ntuples < nloops)
> +                    appendStringInfo(es->str," rows=%.2f loops=%.2f)", rows, nloops);
>
Sorry, but why do the 'loops' need to be changed? IIUC the issue is
only with the
rows field? When testing this patch I got some results similar with this:
    Index Scan using t_a_idx on t  (cost=0.14..0.28 rows=1 width=8)
(actual time=0.049..0.049 rows=0.50 loops=2.00)

> When the total number of returned tuples is less than the number of
> loops currently shows 'rows = 0'. This can mislead users into thinking
> that no rows were returned at all, even though some might have appeared
> occasionally.
>
I think that this can happen when the returned rows and the loops are small
enough to result in a 'row' value like 0.00045? I'm not sure if we have
"bigger" values (e.g 1074(ntuples) / 117(nloops) which would result in 9.17
rows) this would also be true, what do you think? If you could provide
an example of this case would be great!

-    executing the index scans on <literal>tenk2</literal>.
+    executing the index scans on <literal>tenk2</literal>.   If a subplan node
+    is executed multiple times and the average number of rows is less than one,
+    the rows and <literal>loops</literal> values are shown as a
decimal fraction
+    (with two digits after the decimal point) to indicate that some rows
+    were actually processed rather than simply rounding down to zero.

* I think that it would be good to mention what a 'row' value in
decimal means. For
 example, if its says "0.1 rows" the user should assume that typically 0 rows
 will be returned but sometimes it can return 1 or more.

* There are more spaces than necessary before "If a subplan node ..."

* Maybe wrap 'rows' with <literal> </literal>?

-- 
Matheus Alcantara






^ permalink  raw  reply  [nested|flat] 26+ messages in thread

* Re: explain analyze rows=%.0f
  2025-01-13 20:18 Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
  2025-02-07 19:59 ` Re: explain analyze rows=%.0f Matheus Alcantara <[email protected]>
  2025-02-07 20:41   ` Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
  2025-02-07 21:01     ` Re: explain analyze rows=%.0f Matheus Alcantara <[email protected]>
  2025-02-07 21:28       ` Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
  2025-02-10 15:32         ` Re: explain analyze rows=%.0f Matheus Alcantara <[email protected]>
@ 2025-02-10 16:38           ` Ilia Evdokimov <[email protected]>
  2025-02-10 20:43             ` Re: explain analyze rows=%.0f Matheus Alcantara <[email protected]>
  0 siblings, 1 reply; 26+ messages in thread

From: Ilia Evdokimov @ 2025-02-10 16:38 UTC (permalink / raw)
  To: Matheus Alcantara <[email protected]>; +Cc: Guillaume Lelarge <[email protected]>; Daniel Gustafsson <[email protected]>; Ibrar Ahmed <[email protected]>; Gregory Stark (as CFM) <[email protected]>; Tom Lane <[email protected]>; Robert Haas <[email protected]>; Amit Kapila <[email protected]>; Justin Pryzby <[email protected]>; Peter Geoghegan <[email protected]>; vignesh C <[email protected]>; David G. Johnston <[email protected]>; [email protected]


On 10.02.2025 18:32, Matheus Alcantara wrote:
> Thanks for the new patch version.
>
> -- v7-0001-Clarify-display-of-rows-and-loops-as-decimal-fraction.patch
>> +                if (nloops > 1 && planstate->instrument->ntuples < nloops)
>> +                    appendStringInfo(es->str," rows=%.2f loops=%.2f)", rows, nloops);
>>
> Sorry, but why do the 'loops' need to be changed? IIUC the issue is
> only with the
> rows field? When testing this patch I got some results similar with this:
>      Index Scan using t_a_idx on t  (cost=0.14..0.28 rows=1 width=8)
> (actual time=0.049..0.049 rows=0.50 loops=2.00)


The only reason I added this was to make it clear to the user that loops 
 > 1, along with the 'rows' value. If no one finds it useful and it only 
raises more questions, I can remove it.



>
>> When the total number of returned tuples is less than the number of
>> loops currently shows 'rows = 0'. This can mislead users into thinking
>> that no rows were returned at all, even though some might have appeared
>> occasionally.
>>
> I think that this can happen when the returned rows and the loops are small
> enough to result in a 'row' value like 0.00045? I'm not sure if we have
> "bigger" values (e.g 1074(ntuples) / 117(nloops) which would result in 9.17
> rows) this would also be true, what do you think? If you could provide
> an example of this case would be great!


Based on what was discussed earlier in the thread, there are cases with 
large loops [0]. However, I believe it's better not to display average 
rows with excessively long digits or in scientific notation. And, of 
course, I agree with you regarding small values. I think we should also 
add a check to ensure that the total rows is actually greater than zero. 
When the total rows is zero, we could simply display it as an integer 
without decimals. It could help users average rows is very small but not 
zero. What do you think about this approach?


>
> -    executing the index scans on <literal>tenk2</literal>.
> +    executing the index scans on <literal>tenk2</literal>.   If a subplan node
> +    is executed multiple times and the average number of rows is less than one,
> +    the rows and <literal>loops</literal> values are shown as a
> decimal fraction
> +    (with two digits after the decimal point) to indicate that some rows
> +    were actually processed rather than simply rounding down to zero.
>
> * I think that it would be good to mention what a 'row' value in
> decimal means. For
>   example, if its says "0.1 rows" the user should assume that typically 0 rows
>   will be returned but sometimes it can return 1 or more.
>
> * There are more spaces than necessary before "If a subplan node ..."
>
> * Maybe wrap 'rows' with <literal> </literal>?
>

I agree with the last two points. As for the first one—maybe we could 
simply state that the average rows value can be decimal, especially for 
very small values?


[0]: 
https://www.postgresql.org/message-id/a9393107-6bb9-c835-50b7-c0f453a514b8%40postgrespro.ru

--
Best regards,
Ilia Evdokimov,
Tantor Labs LLC.







^ permalink  raw  reply  [nested|flat] 26+ messages in thread

* Re: explain analyze rows=%.0f
  2025-01-13 20:18 Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
  2025-02-07 19:59 ` Re: explain analyze rows=%.0f Matheus Alcantara <[email protected]>
  2025-02-07 20:41   ` Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
  2025-02-07 21:01     ` Re: explain analyze rows=%.0f Matheus Alcantara <[email protected]>
  2025-02-07 21:28       ` Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
  2025-02-10 15:32         ` Re: explain analyze rows=%.0f Matheus Alcantara <[email protected]>
  2025-02-10 16:38           ` Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
@ 2025-02-10 20:43             ` Matheus Alcantara <[email protected]>
  2025-02-10 21:14               ` Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
  0 siblings, 1 reply; 26+ messages in thread

From: Matheus Alcantara @ 2025-02-10 20:43 UTC (permalink / raw)
  To: Ilia Evdokimov <[email protected]>; +Cc: Guillaume Lelarge <[email protected]>; Daniel Gustafsson <[email protected]>; Ibrar Ahmed <[email protected]>; Gregory Stark (as CFM) <[email protected]>; Tom Lane <[email protected]>; Robert Haas <[email protected]>; Amit Kapila <[email protected]>; Justin Pryzby <[email protected]>; Peter Geoghegan <[email protected]>; vignesh C <[email protected]>; David G. Johnston <[email protected]>; [email protected]

Em seg., 10 de fev. de 2025 às 13:38, Ilia Evdokimov
<[email protected]> escreveu:
> > -- v7-0001-Clarify-display-of-rows-and-loops-as-decimal-fraction.patch
> >> +                if (nloops > 1 && planstate->instrument->ntuples < nloops)
> >> +                    appendStringInfo(es->str," rows=%.2f loops=%.2f)", rows, nloops);
> >>
> > Sorry, but why do the 'loops' need to be changed? IIUC the issue is
> > only with the
> > rows field? When testing this patch I got some results similar with this:
> >      Index Scan using t_a_idx on t  (cost=0.14..0.28 rows=1 width=8)
> > (actual time=0.049..0.049 rows=0.50 loops=2.00)
>
>
> The only reason I added this was to make it clear to the user that loops
>  > 1, along with the 'rows' value. If no one finds it useful and it only
> raises more questions, I can remove it.
>
Ok, I get it. IMHO the current behaviour of using %.0f for 'loops' would be
better, but let's see if anyone else has opinions about it.

> >> When the total number of returned tuples is less than the number of
> >> loops currently shows 'rows = 0'. This can mislead users into thinking
> >> that no rows were returned at all, even though some might have appeared
> >> occasionally.
> >>
> > I think that this can happen when the returned rows and the loops are small
> > enough to result in a 'row' value like 0.00045? I'm not sure if we have
> > "bigger" values (e.g 1074(ntuples) / 117(nloops) which would result in 9.17
> > rows) this would also be true, what do you think? If you could provide
> > an example of this case would be great!
>
>
> Based on what was discussed earlier in the thread, there are cases with
> large loops [0]. However, I believe it's better not to display average
> rows with excessively long digits or in scientific notation. And, of
> course, I agree with you regarding small values. I think we should also
> add a check to ensure that the total rows is actually greater than zero.
> When the total rows is zero, we could simply display it as an integer
> without decimals. It could help users average rows is very small but not
> zero. What do you think about this approach?
>
Yeah, I agree with you about the long digits. My question is more about why do
we need the planstate->instrument->ntuples < nloops check? I tried to remove
this check and I got a lot of EXPLAIN output that shows 'rows' values with .00,
so I'm just trying to understand the reason. From what I've understood about
this thread is that just avoiding .00 decimals of 'rows' values that could be
just integers would be enough, is that right or I'm missing something here? I'm
just worried if we could have a scenario where nloops > 1 &&
planstate->instrument->ntuples < nloops which would make the 'rows' not be
formatted correctly.

>
> >
> > -    executing the index scans on <literal>tenk2</literal>.
> > +    executing the index scans on <literal>tenk2</literal>.   If a subplan node
> > +    is executed multiple times and the average number of rows is less than one,
> > +    the rows and <literal>loops</literal> values are shown as a
> > decimal fraction
> > +    (with two digits after the decimal point) to indicate that some rows
> > +    were actually processed rather than simply rounding down to zero.
> >
> > * I think that it would be good to mention what a 'row' value in
> > decimal means. For
> >   example, if its says "0.1 rows" the user should assume that typically 0 rows
> >   will be returned but sometimes it can return 1 or more.
> >
> > * There are more spaces than necessary before "If a subplan node ..."
> >
> > * Maybe wrap 'rows' with <literal> </literal>?
> >
>
> I agree with the last two points. As for the first one—maybe we could
> simply state that the average rows value can be decimal, especially for
> very small values?
>
I'm just not sure about the "small values"; the 'rows' in decimal will only
happen with small values? What would be a "small value" in this context? My main
point here is more that I think that it would be good to mention *why* the
'rows' can be decimal, not just describe that it could be decimal.

-- 
Matheus Alcantara






^ permalink  raw  reply  [nested|flat] 26+ messages in thread

* Re: explain analyze rows=%.0f
  2025-01-13 20:18 Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
  2025-02-07 19:59 ` Re: explain analyze rows=%.0f Matheus Alcantara <[email protected]>
  2025-02-07 20:41   ` Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
  2025-02-07 21:01     ` Re: explain analyze rows=%.0f Matheus Alcantara <[email protected]>
  2025-02-07 21:28       ` Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
  2025-02-10 15:32         ` Re: explain analyze rows=%.0f Matheus Alcantara <[email protected]>
  2025-02-10 16:38           ` Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
  2025-02-10 20:43             ` Re: explain analyze rows=%.0f Matheus Alcantara <[email protected]>
@ 2025-02-10 21:14               ` Ilia Evdokimov <[email protected]>
  2025-02-11 14:51                 ` Re: explain analyze rows=%.0f Matheus Alcantara <[email protected]>
  0 siblings, 1 reply; 26+ messages in thread

From: Ilia Evdokimov @ 2025-02-10 21:14 UTC (permalink / raw)
  To: Matheus Alcantara <[email protected]>; +Cc: Guillaume Lelarge <[email protected]>; Daniel Gustafsson <[email protected]>; Ibrar Ahmed <[email protected]>; Gregory Stark (as CFM) <[email protected]>; Tom Lane <[email protected]>; Robert Haas <[email protected]>; Amit Kapila <[email protected]>; Justin Pryzby <[email protected]>; Peter Geoghegan <[email protected]>; vignesh C <[email protected]>; David G. Johnston <[email protected]>; [email protected]


On 10.02.2025 23:43, Matheus Alcantara wrote:
>>>> When the total number of returned tuples is less than the number of
>>>> loops currently shows 'rows = 0'. This can mislead users into thinking
>>>> that no rows were returned at all, even though some might have appeared
>>>> occasionally.
>>>>
>>> I think that this can happen when the returned rows and the loops are small
>>> enough to result in a 'row' value like 0.00045? I'm not sure if we have
>>> "bigger" values (e.g 1074(ntuples) / 117(nloops) which would result in 9.17
>>> rows) this would also be true, what do you think? If you could provide
>>> an example of this case would be great!
>>
>> Based on what was discussed earlier in the thread, there are cases with
>> large loops [0]. However, I believe it's better not to display average
>> rows with excessively long digits or in scientific notation. And, of
>> course, I agree with you regarding small values. I think we should also
>> add a check to ensure that the total rows is actually greater than zero.
>> When the total rows is zero, we could simply display it as an integer
>> without decimals. It could help users average rows is very small but not
>> zero. What do you think about this approach?
>>
> Yeah, I agree with you about the long digits. My question is more about why do
> we need the planstate->instrument->ntuples < nloops check? I tried to remove
> this check and I got a lot of EXPLAIN output that shows 'rows' values with .00,
> so I'm just trying to understand the reason. From what I've understood about
> this thread is that just avoiding .00 decimals of 'rows' values that could be
> just integers would be enough, is that right or I'm missing something here? I'm
> just worried if we could have a scenario where nloops > 1 &&
> planstate->instrument->ntuples < nloops which would make the 'rows' not be
> formatted correctly.


Sorry for missing your question earlier. If you notice in the code 
above, the variable(average) 'rows' is defined as:

double rows = planstate->instrument->ntuples / nloops;

This represents the total rows divided by the number of loops. The 
condition means that variable 'rows' will always  between zero and one. 
Therefore, the average rows under such conditions cannot be greater than 
or even equal to one. I wrote this condition specifically to avoid the 
verbose expression 'rows > 0 && rows < 1'. However, since this might not 
be obvious to everyone, perhaps it'd be better to write is using 'rows' 
directly or add a comment explaining this logic.


>>> -    executing the index scans on <literal>tenk2</literal>.
>>> +    executing the index scans on <literal>tenk2</literal>.   If a subplan node
>>> +    is executed multiple times and the average number of rows is less than one,
>>> +    the rows and <literal>loops</literal> values are shown as a
>>> decimal fraction
>>> +    (with two digits after the decimal point) to indicate that some rows
>>> +    were actually processed rather than simply rounding down to zero.
>>>
>>> * I think that it would be good to mention what a 'row' value in
>>> decimal means. For
>>>    example, if its says "0.1 rows" the user should assume that typically 0 rows
>>>    will be returned but sometimes it can return 1 or more.
>>>
>>> * There are more spaces than necessary before "If a subplan node ..."
>>>
>>> * Maybe wrap 'rows' with <literal> </literal>?
>>>
>> I agree with the last two points. As for the first one—maybe we could
>> simply state that the average rows value can be decimal, especially for
>> very small values?
>>
> I'm just not sure about the "small values"; the 'rows' in decimal will only
> happen with small values? What would be a "small value" in this context? My main
> point here is more that I think that it would be good to mention *why* the
> 'rows' can be decimal, not just describe that it could be decimal.
>

As for 'small values', it means that the average rows is between zero 
and one, to avoid rounding errors and misunderstanding. I think this 
would be ideal.

--
Best regards,
Ilia Evdokimov,
Tantor Labs LLC.


^ permalink  raw  reply  [nested|flat] 26+ messages in thread

* Re: explain analyze rows=%.0f
  2025-01-13 20:18 Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
  2025-02-07 19:59 ` Re: explain analyze rows=%.0f Matheus Alcantara <[email protected]>
  2025-02-07 20:41   ` Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
  2025-02-07 21:01     ` Re: explain analyze rows=%.0f Matheus Alcantara <[email protected]>
  2025-02-07 21:28       ` Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
  2025-02-10 15:32         ` Re: explain analyze rows=%.0f Matheus Alcantara <[email protected]>
  2025-02-10 16:38           ` Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
  2025-02-10 20:43             ` Re: explain analyze rows=%.0f Matheus Alcantara <[email protected]>
  2025-02-10 21:14               ` Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
@ 2025-02-11 14:51                 ` Matheus Alcantara <[email protected]>
  0 siblings, 0 replies; 26+ messages in thread

From: Matheus Alcantara @ 2025-02-11 14:51 UTC (permalink / raw)
  To: Ilia Evdokimov <[email protected]>; +Cc: Guillaume Lelarge <[email protected]>; Daniel Gustafsson <[email protected]>; Ibrar Ahmed <[email protected]>; Gregory Stark (as CFM) <[email protected]>; Tom Lane <[email protected]>; Robert Haas <[email protected]>; Amit Kapila <[email protected]>; Justin Pryzby <[email protected]>; Peter Geoghegan <[email protected]>; vignesh C <[email protected]>; David G. Johnston <[email protected]>; [email protected]

Em seg., 10 de fev. de 2025 às 18:14, Ilia Evdokimov
<[email protected]> escreveu:
> Sorry for missing your question earlier. If you notice in the code above, the variable(average) 'rows' is defined as:
>
> double rows = planstate->instrument->ntuples / nloops;
>
> This represents the total rows divided by the number of loops. The condition
> means that variable 'rows' will always  between zero and one. Therefore, the
> average rows under such conditions cannot be greater than or even equal to
> one. I wrote this condition specifically to avoid the verbose expression
> 'rows > 0 && rows < 1'. However, since this might not be obvious to everyone,
> perhaps it'd be better to write is using 'rows' directly or add a comment
> explaining this logic.
>
Thanks for the details! It makes sense to me now. I think that adding a comment
could be a good idea

> I agree with the last two points. As for the first one—maybe we could
> simply state that the average rows value can be decimal, especially for
> very small values?
>
> I'm just not sure about the "small values"; the 'rows' in decimal will only
> happen with small values? What would be a "small value" in this context? My main
> point here is more that I think that it would be good to mention *why* the
> 'rows' can be decimal, not just describe that it could be decimal.
>
>
> As for 'small values', it means that the average rows is between zero and
> one, to avoid rounding errors and misunderstanding. I think this would be
> ideal.
>
Get it, sounds reasonable to me.

-- 
Matheus Alcantara






^ permalink  raw  reply  [nested|flat] 26+ messages in thread

* Re: explain analyze rows=%.0f
  2025-01-13 20:18 Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
  2025-02-07 19:59 ` Re: explain analyze rows=%.0f Matheus Alcantara <[email protected]>
  2025-02-07 20:41   ` Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
  2025-02-07 21:01     ` Re: explain analyze rows=%.0f Matheus Alcantara <[email protected]>
  2025-02-07 21:28       ` Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
@ 2025-02-11 17:14         ` Andrei Lepikhov <[email protected]>
  2025-02-11 17:41           ` Re: explain analyze rows=%.0f Robert Haas <[email protected]>
  1 sibling, 1 reply; 26+ messages in thread

From: Andrei Lepikhov @ 2025-02-11 17:14 UTC (permalink / raw)
  To: Ilia Evdokimov <[email protected]>; Matheus Alcantara <[email protected]>; +Cc: Guillaume Lelarge <[email protected]>; Daniel Gustafsson <[email protected]>; Ibrar Ahmed <[email protected]>; Gregory Stark (as CFM) <[email protected]>; Tom Lane <[email protected]>; Robert Haas <[email protected]>; Amit Kapila <[email protected]>; Justin Pryzby <[email protected]>; Peter Geoghegan <[email protected]>; vignesh C <[email protected]>; David G. Johnston <[email protected]>; [email protected]

On 8/2/2025 04:28, Ilia Evdokimov wrote:
> On 08.02.2025 00:01, Matheus Alcantara wrote:
>> Just for reference I'm trying to apply based on commit fb056564ec5.
> You are right, because two commits were appeared after creating v6-patch 
> on partition_prune.out and patch v6 must not have applied on master. 
> Then I created v7 patch rebased on fb056564ec5 . Thank for your remark!
I support the idea in general, but I believe it should be expanded to 
cover all cases of parameterised plan nodes. Each rescan iteration may 
produce a different number of tuples, and rounding can obscure important 
data.

For example, consider five loops of a scan node: the first loop returns 
nine tuples, and each other - zero tuples. When we calculate the 
average, 9 divided by 5 equals 1.8. This results in an explanation that 
indicates "rows = 1," masking almost 40% of the data.

Now, if we apply the same two loops but have a total of 900,000 tuples, 
then 400,000 masked tuples represent a significant portion of the data.

Moreover, switching to a floating-point type for row explanations in 
each parameterised node would provide a more comprehensive view and add 
valuable information about the parameterisation of the node, which may 
not be immediately apparent.

-- 
regards, Andrei Lepikhov






^ permalink  raw  reply  [nested|flat] 26+ messages in thread

* Re: explain analyze rows=%.0f
  2025-01-13 20:18 Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
  2025-02-07 19:59 ` Re: explain analyze rows=%.0f Matheus Alcantara <[email protected]>
  2025-02-07 20:41   ` Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
  2025-02-07 21:01     ` Re: explain analyze rows=%.0f Matheus Alcantara <[email protected]>
  2025-02-07 21:28       ` Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
  2025-02-11 17:14         ` Re: explain analyze rows=%.0f Andrei Lepikhov <[email protected]>
@ 2025-02-11 17:41           ` Robert Haas <[email protected]>
  2025-02-11 19:18             ` Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
  2025-02-11 20:19             ` Re: explain analyze rows=%.0f Andrei Lepikhov <[email protected]>
  2025-02-11 20:46             ` Re: explain analyze rows=%.0f Tom Lane <[email protected]>
  0 siblings, 3 replies; 26+ messages in thread

From: Robert Haas @ 2025-02-11 17:41 UTC (permalink / raw)
  To: Andrei Lepikhov <[email protected]>; +Cc: Ilia Evdokimov <[email protected]>; Matheus Alcantara <[email protected]>; Guillaume Lelarge <[email protected]>; Daniel Gustafsson <[email protected]>; Ibrar Ahmed <[email protected]>; Gregory Stark (as CFM) <[email protected]>; Tom Lane <[email protected]>; Amit Kapila <[email protected]>; Justin Pryzby <[email protected]>; Peter Geoghegan <[email protected]>; vignesh C <[email protected]>; David G. Johnston <[email protected]>; [email protected]

On Tue, Feb 11, 2025 at 12:14 PM Andrei Lepikhov <[email protected]> wrote:
> I support the idea in general, but I believe it should be expanded to
> cover all cases of parameterised plan nodes. Each rescan iteration may
> produce a different number of tuples, and rounding can obscure important
> data.
>
> For example, consider five loops of a scan node: the first loop returns
> nine tuples, and each other - zero tuples. When we calculate the
> average, 9 divided by 5 equals 1.8. This results in an explanation that
> indicates "rows = 1," masking almost 40% of the data.
>
> Now, if we apply the same two loops but have a total of 900,000 tuples,
> then 400,000 masked tuples represent a significant portion of the data.
>
> Moreover, switching to a floating-point type for row explanations in
> each parameterised node would provide a more comprehensive view and add
> valuable information about the parameterisation of the node, which may
> not be immediately apparent.

I agree strongly with all of this. I believe we should just implement
what was agreed here:

https://www.postgresql.org/message-id/21013.1243618236%40sss.pgh.pa.us

Let's just display 2 fractional digits when nloops>1, else 0, and call it good.

-- 
Robert Haas
EDB: http://www.enterprisedb.com






^ permalink  raw  reply  [nested|flat] 26+ messages in thread

* Re: explain analyze rows=%.0f
  2025-01-13 20:18 Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
  2025-02-07 19:59 ` Re: explain analyze rows=%.0f Matheus Alcantara <[email protected]>
  2025-02-07 20:41   ` Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
  2025-02-07 21:01     ` Re: explain analyze rows=%.0f Matheus Alcantara <[email protected]>
  2025-02-07 21:28       ` Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
  2025-02-11 17:14         ` Re: explain analyze rows=%.0f Andrei Lepikhov <[email protected]>
  2025-02-11 17:41           ` Re: explain analyze rows=%.0f Robert Haas <[email protected]>
@ 2025-02-11 19:18             ` Ilia Evdokimov <[email protected]>
  2025-02-11 19:43               ` Re: explain analyze rows=%.0f Alena Rybakina <[email protected]>
  2025-02-12 18:11               ` Re: explain analyze rows=%.0f Robert Haas <[email protected]>
  2 siblings, 2 replies; 26+ messages in thread

From: Ilia Evdokimov @ 2025-02-11 19:18 UTC (permalink / raw)
  To: Robert Haas <[email protected]>; Andrei Lepikhov <[email protected]>; +Cc: Matheus Alcantara <[email protected]>; Guillaume Lelarge <[email protected]>; Daniel Gustafsson <[email protected]>; Ibrar Ahmed <[email protected]>; Gregory Stark (as CFM) <[email protected]>; Tom Lane <[email protected]>; Amit Kapila <[email protected]>; Justin Pryzby <[email protected]>; Peter Geoghegan <[email protected]>; vignesh C <[email protected]>; David G. Johnston <[email protected]>; [email protected]


On 11.02.2025 20:41, Robert Haas wrote:
> On Tue, Feb 11, 2025 at 12:14 PM Andrei Lepikhov <[email protected]> wrote:
>> I support the idea in general, but I believe it should be expanded to
>> cover all cases of parameterised plan nodes. Each rescan iteration may
>> produce a different number of tuples, and rounding can obscure important
>> data.
>>
>> For example, consider five loops of a scan node: the first loop returns
>> nine tuples, and each other - zero tuples. When we calculate the
>> average, 9 divided by 5 equals 1.8. This results in an explanation that
>> indicates "rows = 1," masking almost 40% of the data.
>>
>> Now, if we apply the same two loops but have a total of 900,000 tuples,
>> then 400,000 masked tuples represent a significant portion of the data.
>>
>> Moreover, switching to a floating-point type for row explanations in
>> each parameterised node would provide a more comprehensive view and add
>> valuable information about the parameterisation of the node, which may
>> not be immediately apparent.
> I agree strongly with all of this. I believe we should just implement
> what was agreed here:
>
> https://www.postgresql.org/message-id/21013.1243618236%40sss.pgh.pa.us
>
> Let's just display 2 fractional digits when nloops>1, else 0, and call it good.
>

Thank you for your review!

With such example, it's hard to disagree with it. This would really add 
valuable information. Taking all opinions into account, I have updated 
the patch v8. I have also included a check for the case where there are 
only zeros after the decimal point. We do not want to clutter the rows 
with unnecessary zeros.

--
Best regards,
Ilia Evdokimov,
Tantor Labs LLC.


Attachments:

  [text/x-patch] v8-0001-Clarify-display-of-rows-as-decimal-fractions.patch (8.4K, ../../[email protected]/2-v8-0001-Clarify-display-of-rows-as-decimal-fractions.patch)
  download | inline diff:
From e1a46d3fa5a4d20b6ea4728b2a3955f375539295 Mon Sep 17 00:00:00 2001
From: Ilia Evdokimov <[email protected]>
Date: Tue, 11 Feb 2025 22:12:41 +0300
Subject: [PATCH] Clarify display of rows as decimal fractions

When loops > 1, the average rows value is shown as decimal fractions
with two digits after the decimal point in EXPLAIN ANALYZE.
---
 doc/src/sgml/perform.sgml                     |  2 +
 src/backend/commands/explain.c                | 55 +++++++++++++------
 src/test/regress/expected/partition_prune.out | 18 +++---
 src/test/regress/sql/partition_prune.sql      |  2 +-
 4 files changed, 51 insertions(+), 26 deletions(-)

diff --git a/doc/src/sgml/perform.sgml b/doc/src/sgml/perform.sgml
index a502a2aaba..a01832040c 100644
--- a/doc/src/sgml/perform.sgml
+++ b/doc/src/sgml/perform.sgml
@@ -758,6 +758,8 @@ WHERE t1.unique1 &lt; 10 AND t1.unique2 = t2.unique2;
     the <literal>loops</literal> value to get the total time actually spent in
     the node.  In the above example, we spent a total of 0.030 milliseconds
     executing the index scans on <literal>tenk2</literal>.
+    If <literal>loops</literal> is greater than 1,
+    the <literal>rows</literal> is shown as a decimal unless it's an integer.
    </para>
 
    <para>
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index c24e66f82e..ae81aeea13 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"
@@ -69,6 +71,9 @@ typedef struct SerializeMetrics
  */
 #define BYTES_TO_KILOBYTES(b) (((b) + 1023) / 1024)
 
+/* 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,
 							ParseState *pstate, ParamListInfo params);
@@ -1981,14 +1986,15 @@ ExplainNode(PlanState *planstate, List *ancestors,
 
 		if (es->format == EXPLAIN_FORMAT_TEXT)
 		{
+			appendStringInfo(es->str, " (actual ");
+
 			if (es->timing)
-				appendStringInfo(es->str,
-								 " (actual time=%.3f..%.3f rows=%.0f loops=%.0f)",
-								 startup_ms, total_ms, rows, nloops);
+				appendStringInfo(es->str, "time=%.3f..%.3f ", startup_ms, total_ms);
+
+			if (nloops > 1 && HAS_DECIMAL(rows))
+				appendStringInfo(es->str, "rows=%.2f loops=%.0f)", rows, nloops);
 			else
-				appendStringInfo(es->str,
-								 " (actual rows=%.0f loops=%.0f)",
-								 rows, nloops);
+				appendStringInfo(es->str, "rows=%.0f loops=%.0f)", rows, nloops);
 		}
 		else
 		{
@@ -1999,8 +2005,16 @@ ExplainNode(PlanState *planstate, List *ancestors,
 				ExplainPropertyFloat("Actual Total Time", "ms", total_ms,
 									 3, es);
 			}
-			ExplainPropertyFloat("Actual Rows", NULL, rows, 0, es);
-			ExplainPropertyFloat("Actual Loops", NULL, nloops, 0, es);
+			if (nloops > 1 && HAS_DECIMAL(rows))
+			{
+				ExplainPropertyFloat("Actual Rows", NULL, rows, 2, es);
+				ExplainPropertyFloat("Actual Loops", NULL, nloops, 0, es);
+			}
+			else
+			{
+				ExplainPropertyFloat("Actual Rows", NULL, rows, 0, es);
+				ExplainPropertyFloat("Actual Loops", NULL, nloops, 0, es);
+			}
 		}
 	}
 	else if (es->analyze)
@@ -2052,14 +2066,14 @@ ExplainNode(PlanState *planstate, List *ancestors,
 			if (es->format == EXPLAIN_FORMAT_TEXT)
 			{
 				ExplainIndentText(es);
+				appendStringInfo(es->str, "actual ");
 				if (es->timing)
-					appendStringInfo(es->str,
-									 "actual time=%.3f..%.3f rows=%.0f loops=%.0f\n",
-									 startup_ms, total_ms, rows, nloops);
+					appendStringInfo(es->str, "time=%.3f..%.3f", startup_ms, total_ms);
+
+				if (nloops > 1 && HAS_DECIMAL(rows))
+					appendStringInfo(es->str, "rows=%.2f loops=%.0f\n", rows, nloops);
 				else
-					appendStringInfo(es->str,
-									 "actual rows=%.0f loops=%.0f\n",
-									 rows, nloops);
+					appendStringInfo(es->str, "rows=%.0f loops=%.0f\n", rows, nloops);
 			}
 			else
 			{
@@ -2070,8 +2084,17 @@ ExplainNode(PlanState *planstate, List *ancestors,
 					ExplainPropertyFloat("Actual Total Time", "ms",
 										 total_ms, 3, es);
 				}
-				ExplainPropertyFloat("Actual Rows", NULL, rows, 0, es);
-				ExplainPropertyFloat("Actual Loops", NULL, nloops, 0, es);
+
+				if (nloops > 1 && HAS_DECIMAL(rows))
+				{
+					ExplainPropertyFloat("Actual Rows", NULL, rows, 2, es);
+					ExplainPropertyFloat("Actual Loops", NULL, nloops, 0, es);
+				}
+				else
+				{
+					ExplainPropertyFloat("Actual Rows", NULL, rows, 0, es);
+					ExplainPropertyFloat("Actual Loops", NULL, nloops, 0, es);
+				}
 			}
 
 			ExplainCloseWorker(n, es);
diff --git a/src/test/regress/expected/partition_prune.out b/src/test/regress/expected/partition_prune.out
index e667503c96..fde44db1a0 100644
--- a/src/test/regress/expected/partition_prune.out
+++ b/src/test/regress/expected/partition_prune.out
@@ -2367,7 +2367,7 @@ begin
             $1)
     loop
         ln := regexp_replace(ln, 'Workers Launched: \d+', 'Workers Launched: N');
-        ln := regexp_replace(ln, 'actual rows=\d+ loops=\d+', 'actual rows=N loops=N');
+        ln := regexp_replace(ln, 'actual rows=\d+(?:\.\d+)? loops=\d+', 'actual rows=N loops=N');
         ln := regexp_replace(ln, 'Rows Removed by Filter: \d+', 'Rows Removed by Filter: N');
         return next ln;
     end loop;
@@ -3049,14 +3049,14 @@ order by tbl1.col1, tprt.col1;
 insert into tbl1 values (1001), (1010), (1011);
 explain (analyze, costs off, summary off, timing off, buffers 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)
@@ -3070,16 +3070,16 @@ select * from tbl1 inner join tprt on tbl1.col1 > tprt.col1;
 
 explain (analyze, costs off, summary off, timing off, buffers 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)
diff --git a/src/test/regress/sql/partition_prune.sql b/src/test/regress/sql/partition_prune.sql
index 730545e86a..58756e0b18 100644
--- a/src/test/regress/sql/partition_prune.sql
+++ b/src/test/regress/sql/partition_prune.sql
@@ -586,7 +586,7 @@ begin
             $1)
     loop
         ln := regexp_replace(ln, 'Workers Launched: \d+', 'Workers Launched: N');
-        ln := regexp_replace(ln, 'actual rows=\d+ loops=\d+', 'actual rows=N loops=N');
+        ln := regexp_replace(ln, 'actual rows=\d+(?:\.\d+)? loops=\d+', 'actual rows=N loops=N');
         ln := regexp_replace(ln, 'Rows Removed by Filter: \d+', 'Rows Removed by Filter: N');
         return next ln;
     end loop;
-- 
2.34.1



^ permalink  raw  reply  [nested|flat] 26+ messages in thread

* Re: explain analyze rows=%.0f
  2025-01-13 20:18 Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
  2025-02-07 19:59 ` Re: explain analyze rows=%.0f Matheus Alcantara <[email protected]>
  2025-02-07 20:41   ` Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
  2025-02-07 21:01     ` Re: explain analyze rows=%.0f Matheus Alcantara <[email protected]>
  2025-02-07 21:28       ` Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
  2025-02-11 17:14         ` Re: explain analyze rows=%.0f Andrei Lepikhov <[email protected]>
  2025-02-11 17:41           ` Re: explain analyze rows=%.0f Robert Haas <[email protected]>
  2025-02-11 19:18             ` Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
@ 2025-02-11 19:43               ` Alena Rybakina <[email protected]>
  1 sibling, 0 replies; 26+ messages in thread

From: Alena Rybakina @ 2025-02-11 19:43 UTC (permalink / raw)
  To: Ilia Evdokimov <[email protected]>; +Cc: Robert Haas <[email protected]>; Andrei Lepikhov <[email protected]>; Matheus Alcantara <[email protected]>; Guillaume Lelarge <[email protected]>; Daniel Gustafsson <[email protected]>; Ibrar Ahmed <[email protected]>; Gregory Stark (as CFM) <[email protected]>; Tom Lane <[email protected]>; Amit Kapila <[email protected]>; Justin Pryzby <[email protected]>; Peter Geoghegan <[email protected]>; vignesh C <[email protected]>; David G. Johnston <[email protected]>; [email protected]

Hi! Thank you for your valuable work on this!

On 11.02.2025 22:18, Ilia Evdokimov wrote:
>
> On 11.02.2025 20:41, Robert Haas wrote:
>> On Tue, Feb 11, 2025 at 12:14 PM Andrei Lepikhov <[email protected]> 
>> wrote:
>>> I support the idea in general, but I believe it should be expanded to
>>> cover all cases of parameterised plan nodes. Each rescan iteration may
>>> produce a different number of tuples, and rounding can obscure 
>>> important
>>> data.
>>>
>>> For example, consider five loops of a scan node: the first loop returns
>>> nine tuples, and each other - zero tuples. When we calculate the
>>> average, 9 divided by 5 equals 1.8. This results in an explanation that
>>> indicates "rows = 1," masking almost 40% of the data.
>>>
>>> Now, if we apply the same two loops but have a total of 900,000 tuples,
>>> then 400,000 masked tuples represent a significant portion of the data.
>>>
>>> Moreover, switching to a floating-point type for row explanations in
>>> each parameterised node would provide a more comprehensive view and add
>>> valuable information about the parameterisation of the node, which may
>>> not be immediately apparent.
>> I agree strongly with all of this. I believe we should just implement
>> what was agreed here:
>>
>> https://www.postgresql.org/message-id/21013.1243618236%40sss.pgh.pa.us
>>
>> Let's just display 2 fractional digits when nloops>1, else 0, and 
>> call it good.
>>
>
> Thank you for your review!
>
> With such example, it's hard to disagree with it. This would really 
> add valuable information. Taking all opinions into account, I have 
> updated the patch v8. I have also included a check for the case where 
> there are only zeros after the decimal point. We do not want to 
> clutter the rows with unnecessary zeros.
>
I looked at the patch and agree with them. I would suggest adding a 
description of how this can help in analyzing the query plans -
I think there is a lack of a description of the reason why this is done 
in the commit message.
I would also add the same to the documentation with an example.

-- 
Regards,
Alena Rybakina
Postgres Professional







^ permalink  raw  reply  [nested|flat] 26+ messages in thread

* Re: explain analyze rows=%.0f
  2025-01-13 20:18 Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
  2025-02-07 19:59 ` Re: explain analyze rows=%.0f Matheus Alcantara <[email protected]>
  2025-02-07 20:41   ` Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
  2025-02-07 21:01     ` Re: explain analyze rows=%.0f Matheus Alcantara <[email protected]>
  2025-02-07 21:28       ` Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
  2025-02-11 17:14         ` Re: explain analyze rows=%.0f Andrei Lepikhov <[email protected]>
  2025-02-11 17:41           ` Re: explain analyze rows=%.0f Robert Haas <[email protected]>
  2025-02-11 19:18             ` Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
@ 2025-02-12 18:11               ` Robert Haas <[email protected]>
  1 sibling, 0 replies; 26+ messages in thread

From: Robert Haas @ 2025-02-12 18:11 UTC (permalink / raw)
  To: Ilia Evdokimov <[email protected]>; +Cc: Andrei Lepikhov <[email protected]>; Matheus Alcantara <[email protected]>; Guillaume Lelarge <[email protected]>; Daniel Gustafsson <[email protected]>; Ibrar Ahmed <[email protected]>; Gregory Stark (as CFM) <[email protected]>; Tom Lane <[email protected]>; Amit Kapila <[email protected]>; Justin Pryzby <[email protected]>; Peter Geoghegan <[email protected]>; vignesh C <[email protected]>; David G. Johnston <[email protected]>; [email protected]

On Tue, Feb 11, 2025 at 2:18 PM Ilia Evdokimov
<[email protected]> wrote:
> With such example, it's hard to disagree with it. This would really add
> valuable information. Taking all opinions into account, I have updated
> the patch v8. I have also included a check for the case where there are
> only zeros after the decimal point. We do not want to clutter the rows
> with unnecessary zeros.

I disagree. We don't do this for any other fractional value we print
in any other part of the system, and I do not think this case should
be some kind of weird exception.

-- 
Robert Haas
EDB: http://www.enterprisedb.com






^ permalink  raw  reply  [nested|flat] 26+ messages in thread

* Re: explain analyze rows=%.0f
  2025-01-13 20:18 Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
  2025-02-07 19:59 ` Re: explain analyze rows=%.0f Matheus Alcantara <[email protected]>
  2025-02-07 20:41   ` Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
  2025-02-07 21:01     ` Re: explain analyze rows=%.0f Matheus Alcantara <[email protected]>
  2025-02-07 21:28       ` Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
  2025-02-11 17:14         ` Re: explain analyze rows=%.0f Andrei Lepikhov <[email protected]>
  2025-02-11 17:41           ` Re: explain analyze rows=%.0f Robert Haas <[email protected]>
@ 2025-02-11 20:19             ` Andrei Lepikhov <[email protected]>
  2 siblings, 0 replies; 26+ messages in thread

From: Andrei Lepikhov @ 2025-02-11 20:19 UTC (permalink / raw)
  To: Robert Haas <[email protected]>; +Cc: Ilia Evdokimov <[email protected]>; Matheus Alcantara <[email protected]>; Guillaume Lelarge <[email protected]>; Daniel Gustafsson <[email protected]>; Ibrar Ahmed <[email protected]>; Gregory Stark (as CFM) <[email protected]>; Tom Lane <[email protected]>; Amit Kapila <[email protected]>; Justin Pryzby <[email protected]>; Peter Geoghegan <[email protected]>; vignesh C <[email protected]>; David G. Johnston <[email protected]>; [email protected]

On 12/2/2025 00:41, Robert Haas wrote:
> On Tue, Feb 11, 2025 at 12:14 PM Andrei Lepikhov <[email protected]> wrote:
>> Moreover, switching to a floating-point type for row explanations in
>> each parameterised node would provide a more comprehensive view and add
>> valuable information about the parameterisation of the node, which may
>> not be immediately apparent.
> 
> I agree strongly with all of this. I believe we should just implement
> what was agreed here:
> 
> https://www.postgresql.org/message-id/21013.1243618236%40sss.pgh.pa.us
> 
> Let's just display 2 fractional digits when nloops>1, else 0, and call it good.
Why are there only two fractional digits?

I reviewed the user reports where we identified issues without 
sufficient data, based on explains only, and typical examples included 
loop numbers ranging from 1E5 to 1E7 and tuples from 1E2 to 1E5. 
Therefore, it may make sense to display fractional digits up to two 
meaningful (non-zero) digits.

-- 
regards, Andrei Lepikhov






^ permalink  raw  reply  [nested|flat] 26+ messages in thread

* Re: explain analyze rows=%.0f
  2025-01-13 20:18 Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
  2025-02-07 19:59 ` Re: explain analyze rows=%.0f Matheus Alcantara <[email protected]>
  2025-02-07 20:41   ` Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
  2025-02-07 21:01     ` Re: explain analyze rows=%.0f Matheus Alcantara <[email protected]>
  2025-02-07 21:28       ` Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
  2025-02-11 17:14         ` Re: explain analyze rows=%.0f Andrei Lepikhov <[email protected]>
  2025-02-11 17:41           ` Re: explain analyze rows=%.0f Robert Haas <[email protected]>
@ 2025-02-11 20:46             ` Tom Lane <[email protected]>
  2025-02-12 08:54               ` Re: explain analyze rows=%.0f Andrei Lepikhov <[email protected]>
  2 siblings, 1 reply; 26+ messages in thread

From: Tom Lane @ 2025-02-11 20:46 UTC (permalink / raw)
  To: Robert Haas <[email protected]>; +Cc: Andrei Lepikhov <[email protected]>; Ilia Evdokimov <[email protected]>; Matheus Alcantara <[email protected]>; Guillaume Lelarge <[email protected]>; Daniel Gustafsson <[email protected]>; Ibrar Ahmed <[email protected]>; Gregory Stark (as CFM) <[email protected]>; Amit Kapila <[email protected]>; Justin Pryzby <[email protected]>; Peter Geoghegan <[email protected]>; vignesh C <[email protected]>; David G. Johnston <[email protected]>; [email protected]

Robert Haas <[email protected]> writes:
> On Tue, Feb 11, 2025 at 12:14 PM Andrei Lepikhov <[email protected]> wrote:
>> I support the idea in general, but I believe it should be expanded to
>> cover all cases of parameterised plan nodes. Each rescan iteration may
>> produce a different number of tuples, and rounding can obscure important
>> data.

> I agree strongly with all of this. I believe we should just implement
> what was agreed here:
> https://www.postgresql.org/message-id/21013.1243618236%40sss.pgh.pa.us
> Let's just display 2 fractional digits when nloops>1, else 0, and call it good.

Note that that formulation has nothing especially to do with
parameterized plan nodes.  Any nestloop inner side would end up
getting shown with fractional rowcounts.  Maybe that's fine.

I suggest that when thinking about what to change here,
you start by considering how you'd adjust the docs at
https://www.postgresql.org/docs/devel/using-explain.html
to explain the new behavior.  If you can't explain it
clearly for users, then maybe it's not such a great idea.

			regards, tom lane






^ permalink  raw  reply  [nested|flat] 26+ messages in thread

* Re: explain analyze rows=%.0f
  2025-01-13 20:18 Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
  2025-02-07 19:59 ` Re: explain analyze rows=%.0f Matheus Alcantara <[email protected]>
  2025-02-07 20:41   ` Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
  2025-02-07 21:01     ` Re: explain analyze rows=%.0f Matheus Alcantara <[email protected]>
  2025-02-07 21:28       ` Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
  2025-02-11 17:14         ` Re: explain analyze rows=%.0f Andrei Lepikhov <[email protected]>
  2025-02-11 17:41           ` Re: explain analyze rows=%.0f Robert Haas <[email protected]>
  2025-02-11 20:46             ` Re: explain analyze rows=%.0f Tom Lane <[email protected]>
@ 2025-02-12 08:54               ` Andrei Lepikhov <[email protected]>
  2025-02-12 10:10                 ` Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
  0 siblings, 1 reply; 26+ messages in thread

From: Andrei Lepikhov @ 2025-02-12 08:54 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; Robert Haas <[email protected]>; +Cc: Ilia Evdokimov <[email protected]>; Matheus Alcantara <[email protected]>; Guillaume Lelarge <[email protected]>; Daniel Gustafsson <[email protected]>; Ibrar Ahmed <[email protected]>; Gregory Stark (as CFM) <[email protected]>; Amit Kapila <[email protected]>; Justin Pryzby <[email protected]>; Peter Geoghegan <[email protected]>; vignesh C <[email protected]>; David G. Johnston <[email protected]>; [email protected]

On 12/2/2025 03:46, Tom Lane wrote:
> Robert Haas <[email protected]> writes:
>> On Tue, Feb 11, 2025 at 12:14 PM Andrei Lepikhov <[email protected]> wrote:
>>> I support the idea in general, but I believe it should be expanded to
>>> cover all cases of parameterised plan nodes. Each rescan iteration may
>>> produce a different number of tuples, and rounding can obscure important
>>> data.
> 
>> I agree strongly with all of this. I believe we should just implement
>> what was agreed here:
>> https://www.postgresql.org/message-id/21013.1243618236%40sss.pgh.pa.us
>> Let's just display 2 fractional digits when nloops>1, else 0, and call it good.
> 
> Note that that formulation has nothing especially to do with
> parameterized plan nodes.  Any nestloop inner side would end up
> getting shown with fractional rowcounts.  Maybe that's fine.
I partly agree with this approach. Playing around a bit, I couldn't 
invent a case where we have different numbers of tuples without 
parameters. But I can imagine it is possible or may be possible in 
future. So, it is not necessary to tangle fractional output with a 
parameterised node.
I'm unsure about the inner subtree of a JOIN - subplan may refer to the 
upper query and process a different number of tuples for every 
evaluation without any JOIN operator.
May we agree on a more general formula to print at least two meaningful 
digits if we have a fractional part?

Examples:
- actual rows = 2, nloops = 2 -> rows = 1
- actual rows = 9, nloops = 5 -> rows = 1.8
- actual rows = 101, nloops = 100 -> rows = 1.0
- actual rows = 99, nloops = 1000000 -> rows = 0.000099

It may guarantee that an EXPLAIN exposes most of the data passed the 
node, enough to tangle it with actual timing and tuples at the upper 
levels of the query.

> 
> I suggest that when thinking about what to change here,
> you start by considering how you'd adjust the docs at
> https://www.postgresql.org/docs/devel/using-explain.html
> to explain the new behavior.  If you can't explain it
> clearly for users, then maybe it's not such a great idea.
Agree

-- 
regards, Andrei Lepikhov






^ permalink  raw  reply  [nested|flat] 26+ messages in thread

* Re: explain analyze rows=%.0f
  2025-01-13 20:18 Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
  2025-02-07 19:59 ` Re: explain analyze rows=%.0f Matheus Alcantara <[email protected]>
  2025-02-07 20:41   ` Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
  2025-02-07 21:01     ` Re: explain analyze rows=%.0f Matheus Alcantara <[email protected]>
  2025-02-07 21:28       ` Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
  2025-02-11 17:14         ` Re: explain analyze rows=%.0f Andrei Lepikhov <[email protected]>
  2025-02-11 17:41           ` Re: explain analyze rows=%.0f Robert Haas <[email protected]>
  2025-02-11 20:46             ` Re: explain analyze rows=%.0f Tom Lane <[email protected]>
  2025-02-12 08:54               ` Re: explain analyze rows=%.0f Andrei Lepikhov <[email protected]>
@ 2025-02-12 10:10                 ` Ilia Evdokimov <[email protected]>
  2025-02-12 18:19                   ` Re: explain analyze rows=%.0f Robert Haas <[email protected]>
  0 siblings, 1 reply; 26+ messages in thread

From: Ilia Evdokimov @ 2025-02-12 10:10 UTC (permalink / raw)
  To: Andrei Lepikhov <[email protected]>; Tom Lane <[email protected]>; Robert Haas <[email protected]>; +Cc: Matheus Alcantara <[email protected]>; Guillaume Lelarge <[email protected]>; Daniel Gustafsson <[email protected]>; Ibrar Ahmed <[email protected]>; Gregory Stark (as CFM) <[email protected]>; Amit Kapila <[email protected]>; Justin Pryzby <[email protected]>; Peter Geoghegan <[email protected]>; vignesh C <[email protected]>; David G. Johnston <[email protected]>; [email protected]


On 12.02.2025 11:54, Andrei Lepikhov wrote:
> May we agree on a more general formula to print at least two 
> meaningful digits if we have a fractional part?
>
> Examples:
> - actual rows = 2, nloops = 2 -> rows = 1
> - actual rows = 9, nloops = 5 -> rows = 1.8
> - actual rows = 101, nloops = 100 -> rows = 1.0
> - actual rows = 99, nloops = 1000000 -> rows = 0.000099
>
> It may guarantee that an EXPLAIN exposes most of the data passed the 
> node, enough to tangle it with actual timing and tuples at the upper 
> levels of the query.

I think the idea of keeping two significant digits after the decimal 
point is quite reasonable. The thing is, rows=0.000001 or something 
similar can only occur when loops is quite large. If we show the order 
of magnitude in rows, it will be easier for the user to estimate the 
order of total rows. For example, if we see this:

rows=0.000056 loops=4718040

the user can quickler approximate the order of total rows for analyzing 
the upper levels of the query.

However, keep in mind that I am against using the E notation, as many 
users have mentioned that they are not mathematicians and are not 
familiar with the concept of "E".


>
>>
>> I suggest that when thinking about what to change here,
>> you start by considering how you'd adjust the docs at
>> https://www.postgresql.org/docs/devel/using-explain.html
>> to explain the new behavior.  If you can't explain it
>> clearly for users, then maybe it's not such a great idea.
> Agree
>
So do I. Firstly, I'll think how to explain it.

--
Best regards,
Ilia Evdokimov,
Tantor Labs LLC.







^ permalink  raw  reply  [nested|flat] 26+ messages in thread

* Re: explain analyze rows=%.0f
  2025-01-13 20:18 Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
  2025-02-07 19:59 ` Re: explain analyze rows=%.0f Matheus Alcantara <[email protected]>
  2025-02-07 20:41   ` Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
  2025-02-07 21:01     ` Re: explain analyze rows=%.0f Matheus Alcantara <[email protected]>
  2025-02-07 21:28       ` Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
  2025-02-11 17:14         ` Re: explain analyze rows=%.0f Andrei Lepikhov <[email protected]>
  2025-02-11 17:41           ` Re: explain analyze rows=%.0f Robert Haas <[email protected]>
  2025-02-11 20:46             ` Re: explain analyze rows=%.0f Tom Lane <[email protected]>
  2025-02-12 08:54               ` Re: explain analyze rows=%.0f Andrei Lepikhov <[email protected]>
  2025-02-12 10:10                 ` Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
@ 2025-02-12 18:19                   ` Robert Haas <[email protected]>
  2025-02-12 18:40                     ` Re: explain analyze rows=%.0f Tom Lane <[email protected]>
  0 siblings, 1 reply; 26+ messages in thread

From: Robert Haas @ 2025-02-12 18:19 UTC (permalink / raw)
  To: Ilia Evdokimov <[email protected]>; +Cc: Andrei Lepikhov <[email protected]>; Tom Lane <[email protected]>; Matheus Alcantara <[email protected]>; Guillaume Lelarge <[email protected]>; Daniel Gustafsson <[email protected]>; Ibrar Ahmed <[email protected]>; Gregory Stark (as CFM) <[email protected]>; Amit Kapila <[email protected]>; Justin Pryzby <[email protected]>; Peter Geoghegan <[email protected]>; vignesh C <[email protected]>; David G. Johnston <[email protected]>; [email protected]

On Wed, Feb 12, 2025 at 5:10 AM Ilia Evdokimov
<[email protected]> wrote:
> I think the idea of keeping two significant digits after the decimal
> point is quite reasonable. The thing is, rows=0.000001 or something
> similar can only occur when loops is quite large. If we show the order
> of magnitude in rows, it will be easier for the user to estimate the
> order of total rows. For example, if we see this:
>
> rows=0.000056 loops=4718040
>
> the user can quickler approximate the order of total rows for analyzing
> the upper levels of the query.

I agree that showing 2 digits after the decimal point in all cases is
not ideal, but I suggest that we take a practical approach. Figuring
out dynamically what number of decimal digits to display in each case
sounds complicated and we may spend a bunch of time arguing about the
details of that and get nothing committed. If we just show 2 digits
after the decimal point, it will not be perfect, but it will be 10^2
times better than what we have now.

If I'm honest, what I actually think we should do is stop dividing
values by nloops before printing them out. Every time I'm looking at a
quantity that has been divided by nloops, the very first thing I do is
try to figure out what the original value was. The whole reason I want
to display at least a couple of decimal digits here is so that I can
do that more accurately, but of course the dream would be not having
to reverse engineer it like that at all. However, I expect fierce
opposition to that idea, and no matter how misguided I may think that
opposition might be, a patch in the tree is worth two in the
CommitFest.

-- 
Robert Haas
EDB: http://www.enterprisedb.com






^ permalink  raw  reply  [nested|flat] 26+ messages in thread

* Re: explain analyze rows=%.0f
  2025-01-13 20:18 Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
  2025-02-07 19:59 ` Re: explain analyze rows=%.0f Matheus Alcantara <[email protected]>
  2025-02-07 20:41   ` Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
  2025-02-07 21:01     ` Re: explain analyze rows=%.0f Matheus Alcantara <[email protected]>
  2025-02-07 21:28       ` Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
  2025-02-11 17:14         ` Re: explain analyze rows=%.0f Andrei Lepikhov <[email protected]>
  2025-02-11 17:41           ` Re: explain analyze rows=%.0f Robert Haas <[email protected]>
  2025-02-11 20:46             ` Re: explain analyze rows=%.0f Tom Lane <[email protected]>
  2025-02-12 08:54               ` Re: explain analyze rows=%.0f Andrei Lepikhov <[email protected]>
  2025-02-12 10:10                 ` Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
  2025-02-12 18:19                   ` Re: explain analyze rows=%.0f Robert Haas <[email protected]>
@ 2025-02-12 18:40                     ` Tom Lane <[email protected]>
  2025-02-12 19:55                       ` Re: explain analyze rows=%.0f Andrei Lepikhov <[email protected]>
  0 siblings, 1 reply; 26+ messages in thread

From: Tom Lane @ 2025-02-12 18:40 UTC (permalink / raw)
  To: Robert Haas <[email protected]>; +Cc: Ilia Evdokimov <[email protected]>; Andrei Lepikhov <[email protected]>; Matheus Alcantara <[email protected]>; Guillaume Lelarge <[email protected]>; Daniel Gustafsson <[email protected]>; Ibrar Ahmed <[email protected]>; Gregory Stark (as CFM) <[email protected]>; Amit Kapila <[email protected]>; Justin Pryzby <[email protected]>; Peter Geoghegan <[email protected]>; vignesh C <[email protected]>; David G. Johnston <[email protected]>; [email protected]

Robert Haas <[email protected]> writes:
> I agree that showing 2 digits after the decimal point in all cases is
> not ideal, but I suggest that we take a practical approach. Figuring
> out dynamically what number of decimal digits to display in each case
> sounds complicated and we may spend a bunch of time arguing about the
> details of that and get nothing committed. If we just show 2 digits
> after the decimal point, it will not be perfect, but it will be 10^2
> times better than what we have now.

I was idly speculating yesterday about letting the Ryu code print
the division result, so that we get a variable number of digits.
Realistically, that'd probably result in many cases in more digits
than anybody wants, so it's not a serious proposal.  I'm cool with
the fixed-two-digits approach to start with.

			regards, tom lane






^ permalink  raw  reply  [nested|flat] 26+ messages in thread

* Re: explain analyze rows=%.0f
  2025-01-13 20:18 Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
  2025-02-07 19:59 ` Re: explain analyze rows=%.0f Matheus Alcantara <[email protected]>
  2025-02-07 20:41   ` Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
  2025-02-07 21:01     ` Re: explain analyze rows=%.0f Matheus Alcantara <[email protected]>
  2025-02-07 21:28       ` Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
  2025-02-11 17:14         ` Re: explain analyze rows=%.0f Andrei Lepikhov <[email protected]>
  2025-02-11 17:41           ` Re: explain analyze rows=%.0f Robert Haas <[email protected]>
  2025-02-11 20:46             ` Re: explain analyze rows=%.0f Tom Lane <[email protected]>
  2025-02-12 08:54               ` Re: explain analyze rows=%.0f Andrei Lepikhov <[email protected]>
  2025-02-12 10:10                 ` Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
  2025-02-12 18:19                   ` Re: explain analyze rows=%.0f Robert Haas <[email protected]>
  2025-02-12 18:40                     ` Re: explain analyze rows=%.0f Tom Lane <[email protected]>
@ 2025-02-12 19:55                       ` Andrei Lepikhov <[email protected]>
  2025-02-12 19:56                         ` Re: explain analyze rows=%.0f Robert Haas <[email protected]>
  0 siblings, 1 reply; 26+ messages in thread

From: Andrei Lepikhov @ 2025-02-12 19:55 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; Robert Haas <[email protected]>; +Cc: Ilia Evdokimov <[email protected]>; Matheus Alcantara <[email protected]>; Guillaume Lelarge <[email protected]>; Daniel Gustafsson <[email protected]>; Ibrar Ahmed <[email protected]>; Gregory Stark (as CFM) <[email protected]>; Amit Kapila <[email protected]>; Justin Pryzby <[email protected]>; Peter Geoghegan <[email protected]>; vignesh C <[email protected]>; David G. Johnston <[email protected]>; [email protected]

On 13/2/2025 01:40, Tom Lane wrote:
> I was idly speculating yesterday about letting the Ryu code print
> the division result, so that we get a variable number of digits.
> Realistically, that'd probably result in many cases in more digits
> than anybody wants, so it's not a serious proposal.  I'm cool with
> the fixed-two-digits approach to start with.
Okay, since no one else voted for the meaningful-numbers approach, I 
would say that fixed size is better than nothing. It may cover some of 
my practical cases, but unfortunately, not the most problematic ones.

-- 
regards, Andrei Lepikhov






^ permalink  raw  reply  [nested|flat] 26+ messages in thread

* Re: explain analyze rows=%.0f
  2025-01-13 20:18 Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
  2025-02-07 19:59 ` Re: explain analyze rows=%.0f Matheus Alcantara <[email protected]>
  2025-02-07 20:41   ` Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
  2025-02-07 21:01     ` Re: explain analyze rows=%.0f Matheus Alcantara <[email protected]>
  2025-02-07 21:28       ` Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
  2025-02-11 17:14         ` Re: explain analyze rows=%.0f Andrei Lepikhov <[email protected]>
  2025-02-11 17:41           ` Re: explain analyze rows=%.0f Robert Haas <[email protected]>
  2025-02-11 20:46             ` Re: explain analyze rows=%.0f Tom Lane <[email protected]>
  2025-02-12 08:54               ` Re: explain analyze rows=%.0f Andrei Lepikhov <[email protected]>
  2025-02-12 10:10                 ` Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
  2025-02-12 18:19                   ` Re: explain analyze rows=%.0f Robert Haas <[email protected]>
  2025-02-12 18:40                     ` Re: explain analyze rows=%.0f Tom Lane <[email protected]>
  2025-02-12 19:55                       ` Re: explain analyze rows=%.0f Andrei Lepikhov <[email protected]>
@ 2025-02-12 19:56                         ` Robert Haas <[email protected]>
  2025-02-13 09:05                           ` Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
  0 siblings, 1 reply; 26+ messages in thread

From: Robert Haas @ 2025-02-12 19:56 UTC (permalink / raw)
  To: Andrei Lepikhov <[email protected]>; +Cc: Tom Lane <[email protected]>; Ilia Evdokimov <[email protected]>; Matheus Alcantara <[email protected]>; Guillaume Lelarge <[email protected]>; Daniel Gustafsson <[email protected]>; Ibrar Ahmed <[email protected]>; Gregory Stark (as CFM) <[email protected]>; Amit Kapila <[email protected]>; Justin Pryzby <[email protected]>; Peter Geoghegan <[email protected]>; vignesh C <[email protected]>; David G. Johnston <[email protected]>; [email protected]

On Wed, Feb 12, 2025 at 2:55 PM Andrei Lepikhov <[email protected]> wrote:
> On 13/2/2025 01:40, Tom Lane wrote:
> > I was idly speculating yesterday about letting the Ryu code print
> > the division result, so that we get a variable number of digits.
> > Realistically, that'd probably result in many cases in more digits
> > than anybody wants, so it's not a serious proposal.  I'm cool with
> > the fixed-two-digits approach to start with.
> Okay, since no one else voted for the meaningful-numbers approach, I
> would say that fixed size is better than nothing. It may cover some of
> my practical cases, but unfortunately, not the most problematic ones.

I don't love it either, but I do think it is significantly better than nothing.

-- 
Robert Haas
EDB: http://www.enterprisedb.com






^ permalink  raw  reply  [nested|flat] 26+ messages in thread

* Re: explain analyze rows=%.0f
  2025-01-13 20:18 Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
  2025-02-07 19:59 ` Re: explain analyze rows=%.0f Matheus Alcantara <[email protected]>
  2025-02-07 20:41   ` Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
  2025-02-07 21:01     ` Re: explain analyze rows=%.0f Matheus Alcantara <[email protected]>
  2025-02-07 21:28       ` Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
  2025-02-11 17:14         ` Re: explain analyze rows=%.0f Andrei Lepikhov <[email protected]>
  2025-02-11 17:41           ` Re: explain analyze rows=%.0f Robert Haas <[email protected]>
  2025-02-11 20:46             ` Re: explain analyze rows=%.0f Tom Lane <[email protected]>
  2025-02-12 08:54               ` Re: explain analyze rows=%.0f Andrei Lepikhov <[email protected]>
  2025-02-12 10:10                 ` Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
  2025-02-12 18:19                   ` Re: explain analyze rows=%.0f Robert Haas <[email protected]>
  2025-02-12 18:40                     ` Re: explain analyze rows=%.0f Tom Lane <[email protected]>
  2025-02-12 19:55                       ` Re: explain analyze rows=%.0f Andrei Lepikhov <[email protected]>
  2025-02-12 19:56                         ` Re: explain analyze rows=%.0f Robert Haas <[email protected]>
@ 2025-02-13 09:05                           ` Ilia Evdokimov <[email protected]>
  2025-02-13 20:42                             ` Re: explain analyze rows=%.0f Robert Haas <[email protected]>
  0 siblings, 1 reply; 26+ messages in thread

From: Ilia Evdokimov @ 2025-02-13 09:05 UTC (permalink / raw)
  To: [email protected]; +Cc: Tom Lane <[email protected]>; Matheus Alcantara <[email protected]>; Guillaume Lelarge <[email protected]>; Daniel Gustafsson <[email protected]>; Ibrar Ahmed <[email protected]>; Gregory Stark (as CFM) <[email protected]>; Amit Kapila <[email protected]>; Justin Pryzby <[email protected]>; Peter Geoghegan <[email protected]>; vignesh C <[email protected]>; David G. Johnston <[email protected]>; Alena Rybakina <[email protected]>; Andrei Lepikhov <[email protected]>; Robert Haas <[email protected]>


On 12.02.2025 22:56, Robert Haas wrote:
> On Wed, Feb 12, 2025 at 2:55 PM Andrei Lepikhov<[email protected]>  wrote:
>> On 13/2/2025 01:40, Tom Lane wrote:
>>> I was idly speculating yesterday about letting the Ryu code print
>>> the division result, so that we get a variable number of digits.
>>> Realistically, that'd probably result in many cases in more digits
>>> than anybody wants, so it's not a serious proposal.  I'm cool with
>>> the fixed-two-digits approach to start with.
>> Okay, since no one else voted for the meaningful-numbers approach, I
>> would say that fixed size is better than nothing. It may cover some of
>> my practical cases, but unfortunately, not the most problematic ones.
> I don't love it either, but I do think it is significantly better than nothing.
>


I'm in favor of having some improvement rather than nothing at 
all—otherwise, we might never reach a consensus.

1. Documentation 
(v9-0001-Clarify-display-of-rows-as-decimal-fractions-DOC.patch)

One thing that bothers me is that the documentation explains how to 
compute total time, but it does not clarify how to compute total rows. 
Maybe this was obvious to others before, but now that we are displaying 
|rows| as a fraction, we should explicitly document how to interpret it 
alongside total time.

I believe it would be helpful to show a non-integer rows value in an 
example query. However, to achieve this, we need the index scan results 
to vary across iterations. One way to accomplish this is by using the 
condition t1.unique2 > t2.unique2. Additionally, I suggest making loops 
a round number (e.g., 100) for better readability, which can be achieved 
using t1.thousand < 10. The final query would look like this:

EXPLAIN ANALYZE SELECT *
FROM tenk1 t1, tenk2 t2
WHERE t1.thousand < 10 AND t1.unique2 > t2.unique2;

I believe this is an ideal example for the documentation because it not 
only demonstrates fractional rows, but also keeps the execution plan 
nearly unchanged. While the estimated and actual average row counts 
become slightly rounded, I don't see another way to ensure different 
results for each index scan.

I'm open to any feedback or suggestions for a better example to use in 
the documentation or additional explaining fractional rows in the text.

2. Code and tests 
(v9-0002-Clarify-display-of-rows-as-decimal-fractions.patch)

I left the code and tests unchanged since we agreed on a fixed format of 
two decimal places.

--
Best regards,
Ilia Evdokimov,
Tantor Labs LLC.


Attachments:

  [text/x-patch] v9-0001-Clarify-display-of-rows-as-decimal-fractions-DOC.patch (4.1K, ../../[email protected]/3-v9-0001-Clarify-display-of-rows-as-decimal-fractions-DOC.patch)
  download | inline diff:
From 789d98383988ef6d3b0bc2c6b9b5c73a83ffd6d4 Mon Sep 17 00:00:00 2001
From: Ilia Evdokimov <[email protected]>
Date: Thu, 13 Feb 2025 11:19:03 +0300
Subject: [PATCH v9] Clarify display of rows as decimal fractions

When loops > 1, the average rows value is now displayed as a decimal fraction
with two digits after the decimal point in EXPLAIN ANALYZE.

Previously, the average rows value was always rounded to an integer,
which could lead to misleading results when estimating total rows
by multiplying rows by loop. This change ensures that users
get a more accurate representation of the data flow through execution nodes.
---
 doc/src/sgml/perform.sgml | 41 ++++++++++++++++++++-------------------
 1 file changed, 21 insertions(+), 20 deletions(-)

diff --git a/doc/src/sgml/perform.sgml b/doc/src/sgml/perform.sgml
index a502a2aaba..dd61ae4507 100644
--- a/doc/src/sgml/perform.sgml
+++ b/doc/src/sgml/perform.sgml
@@ -717,26 +717,26 @@ FROM tenk1 t1 WHERE t1.ten = (SELECT (random() * 10)::integer);
 <screen>
 EXPLAIN ANALYZE SELECT *
 FROM tenk1 t1, tenk2 t2
-WHERE t1.unique1 &lt; 10 AND t1.unique2 = t2.unique2;
-
-                                                           QUERY PLAN
--------------------------------------------------------------------&zwsp;--------------------------------------------------------------
- Nested Loop  (cost=4.65..118.50 rows=10 width=488) (actual time=0.017..0.051 rows=10 loops=1)
-   Buffers: shared hit=36 read=6
-   -&gt;  Bitmap Heap Scan on tenk1 t1  (cost=4.36..39.38 rows=10 width=244) (actual time=0.009..0.017 rows=10 loops=1)
-         Recheck Cond: (unique1 &lt; 10)
-         Heap Blocks: exact=10
-         Buffers: shared hit=3 read=5 written=4
-         -&gt;  Bitmap Index Scan on tenk1_unique1  (cost=0.00..4.36 rows=10 width=0) (actual time=0.004..0.004 rows=10 loops=1)
-               Index Cond: (unique1 &lt; 10)
+WHERE t1.thousand &lt; 10 AND t1.unique2 &gt; t2.unique2;
+
+                                                                 QUERY PLAN
+-------------------------------------------------------------------&zwsp;-------------------------------------------------------------------------
+ Nested Loop  (cost=5.40..11571.44 rows=356667 width=488) (actual time=0.042..117.205 rows=513832 loops=1)
+   Buffers: shared hit=19377 read=29
+   -&gt;  Bitmap Heap Scan on tenk1 t1  (cost=5.11..233.60 rows=107 width=244) (actual time=0.021..0.103 rows=100 loops=1)
+         Recheck Cond: (thousand &lt; 10)
+         Heap Blocks: exact=90
+         Buffers: shared hit=92
+         -&gt;  Bitmap Index Scan on tenk1_thous_tenthous  (cost=0.00..5.09 rows=107 width=0) (actual time=0.010..0.010 rows=100 loops=1)
+               Index Cond: (thousand &lt; 10)
                Buffers: shared hit=2
-   -&gt;  Index Scan using tenk2_unique2 on tenk2 t2  (cost=0.29..7.90 rows=1 width=244) (actual time=0.003..0.003 rows=1 loops=10)
-         Index Cond: (unique2 = t1.unique2)
-         Buffers: shared hit=24 read=6
+   -&gt;  Index Scan using tenk2_unique2 on tenk2 t2  (cost=0.29..72.63 rows=3333 width=244) (actual time=0.006..0.410 rows=5138.32 loops=100)
+         Index Cond: (unique2 &lt; t1.unique2)
+         Buffers: shared hit=19285 read=29
  Planning:
-   Buffers: shared hit=15 dirtied=9
- Planning Time: 0.485 ms
- Execution Time: 0.073 ms
+   Buffers: shared hit=258 read=9 dirtied=2
+ Planning Time: 0.519 ms
+ Execution Time: 131.378 ms
 </screen>
 
     Note that the <quote>actual time</quote> values are in milliseconds of
@@ -756,8 +756,9 @@ WHERE t1.unique1 &lt; 10 AND t1.unique2 = t2.unique2;
     values shown are averages per-execution.  This is done to make the numbers
     comparable with the way that the cost estimates are shown.  Multiply by
     the <literal>loops</literal> value to get the total time actually spent in
-    the node.  In the above example, we spent a total of 0.030 milliseconds
-    executing the index scans on <literal>tenk2</literal>.
+    the node, and to get the total rows processed in the node. In the above example,
+    we spent a total of 41 milliseconds executing the index scans,
+    and the node processed a total of 513832 rows on <literal>tenk2</literal>.
    </para>
 
    <para>
-- 
2.34.1



  [text/x-patch] v9-0002-Clarify-display-of-rows-as-decimal-fractions.patch (8.0K, ../../[email protected]/4-v9-0002-Clarify-display-of-rows-as-decimal-fractions.patch)
  download | inline diff:
From a30ee71240d731c386f9dd4eb37b2817e3238807 Mon Sep 17 00:00:00 2001
From: Ilia Evdokimov <[email protected]>
Date: Thu, 13 Feb 2025 11:24:03 +0300
Subject: [PATCH v9] Clarify display of rows as decimal fractions

When loops > 1, the average rows value is now displayed as a decimal fraction
with two digits after the decimal point in EXPLAIN ANALYZE.

Previously, the average rows value was always rounded to an integer,
which could lead to misleading results when estimating total rows
by multiplying rows by loop. This change ensures that users
get a more accurate representation of the data flow through execution nodes.
---
 src/backend/commands/explain.c                | 55 +++++++++++++------
 src/test/regress/expected/partition_prune.out | 18 +++---
 src/test/regress/sql/partition_prune.sql      |  2 +-
 3 files changed, 49 insertions(+), 26 deletions(-)

diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index c24e66f82e..ae81aeea13 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"
@@ -69,6 +71,9 @@ typedef struct SerializeMetrics
  */
 #define BYTES_TO_KILOBYTES(b) (((b) + 1023) / 1024)
 
+/* 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,
 							ParseState *pstate, ParamListInfo params);
@@ -1981,14 +1986,15 @@ ExplainNode(PlanState *planstate, List *ancestors,
 
 		if (es->format == EXPLAIN_FORMAT_TEXT)
 		{
+			appendStringInfo(es->str, " (actual ");
+
 			if (es->timing)
-				appendStringInfo(es->str,
-								 " (actual time=%.3f..%.3f rows=%.0f loops=%.0f)",
-								 startup_ms, total_ms, rows, nloops);
+				appendStringInfo(es->str, "time=%.3f..%.3f ", startup_ms, total_ms);
+
+			if (nloops > 1 && HAS_DECIMAL(rows))
+				appendStringInfo(es->str, "rows=%.2f loops=%.0f)", rows, nloops);
 			else
-				appendStringInfo(es->str,
-								 " (actual rows=%.0f loops=%.0f)",
-								 rows, nloops);
+				appendStringInfo(es->str, "rows=%.0f loops=%.0f)", rows, nloops);
 		}
 		else
 		{
@@ -1999,8 +2005,16 @@ ExplainNode(PlanState *planstate, List *ancestors,
 				ExplainPropertyFloat("Actual Total Time", "ms", total_ms,
 									 3, es);
 			}
-			ExplainPropertyFloat("Actual Rows", NULL, rows, 0, es);
-			ExplainPropertyFloat("Actual Loops", NULL, nloops, 0, es);
+			if (nloops > 1 && HAS_DECIMAL(rows))
+			{
+				ExplainPropertyFloat("Actual Rows", NULL, rows, 2, es);
+				ExplainPropertyFloat("Actual Loops", NULL, nloops, 0, es);
+			}
+			else
+			{
+				ExplainPropertyFloat("Actual Rows", NULL, rows, 0, es);
+				ExplainPropertyFloat("Actual Loops", NULL, nloops, 0, es);
+			}
 		}
 	}
 	else if (es->analyze)
@@ -2052,14 +2066,14 @@ ExplainNode(PlanState *planstate, List *ancestors,
 			if (es->format == EXPLAIN_FORMAT_TEXT)
 			{
 				ExplainIndentText(es);
+				appendStringInfo(es->str, "actual ");
 				if (es->timing)
-					appendStringInfo(es->str,
-									 "actual time=%.3f..%.3f rows=%.0f loops=%.0f\n",
-									 startup_ms, total_ms, rows, nloops);
+					appendStringInfo(es->str, "time=%.3f..%.3f", startup_ms, total_ms);
+
+				if (nloops > 1 && HAS_DECIMAL(rows))
+					appendStringInfo(es->str, "rows=%.2f loops=%.0f\n", rows, nloops);
 				else
-					appendStringInfo(es->str,
-									 "actual rows=%.0f loops=%.0f\n",
-									 rows, nloops);
+					appendStringInfo(es->str, "rows=%.0f loops=%.0f\n", rows, nloops);
 			}
 			else
 			{
@@ -2070,8 +2084,17 @@ ExplainNode(PlanState *planstate, List *ancestors,
 					ExplainPropertyFloat("Actual Total Time", "ms",
 										 total_ms, 3, es);
 				}
-				ExplainPropertyFloat("Actual Rows", NULL, rows, 0, es);
-				ExplainPropertyFloat("Actual Loops", NULL, nloops, 0, es);
+
+				if (nloops > 1 && HAS_DECIMAL(rows))
+				{
+					ExplainPropertyFloat("Actual Rows", NULL, rows, 2, es);
+					ExplainPropertyFloat("Actual Loops", NULL, nloops, 0, es);
+				}
+				else
+				{
+					ExplainPropertyFloat("Actual Rows", NULL, rows, 0, es);
+					ExplainPropertyFloat("Actual Loops", NULL, nloops, 0, es);
+				}
 			}
 
 			ExplainCloseWorker(n, es);
diff --git a/src/test/regress/expected/partition_prune.out b/src/test/regress/expected/partition_prune.out
index e667503c96..fde44db1a0 100644
--- a/src/test/regress/expected/partition_prune.out
+++ b/src/test/regress/expected/partition_prune.out
@@ -2367,7 +2367,7 @@ begin
             $1)
     loop
         ln := regexp_replace(ln, 'Workers Launched: \d+', 'Workers Launched: N');
-        ln := regexp_replace(ln, 'actual rows=\d+ loops=\d+', 'actual rows=N loops=N');
+        ln := regexp_replace(ln, 'actual rows=\d+(?:\.\d+)? loops=\d+', 'actual rows=N loops=N');
         ln := regexp_replace(ln, 'Rows Removed by Filter: \d+', 'Rows Removed by Filter: N');
         return next ln;
     end loop;
@@ -3049,14 +3049,14 @@ order by tbl1.col1, tprt.col1;
 insert into tbl1 values (1001), (1010), (1011);
 explain (analyze, costs off, summary off, timing off, buffers 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)
@@ -3070,16 +3070,16 @@ select * from tbl1 inner join tprt on tbl1.col1 > tprt.col1;
 
 explain (analyze, costs off, summary off, timing off, buffers 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)
diff --git a/src/test/regress/sql/partition_prune.sql b/src/test/regress/sql/partition_prune.sql
index 730545e86a..58756e0b18 100644
--- a/src/test/regress/sql/partition_prune.sql
+++ b/src/test/regress/sql/partition_prune.sql
@@ -586,7 +586,7 @@ begin
             $1)
     loop
         ln := regexp_replace(ln, 'Workers Launched: \d+', 'Workers Launched: N');
-        ln := regexp_replace(ln, 'actual rows=\d+ loops=\d+', 'actual rows=N loops=N');
+        ln := regexp_replace(ln, 'actual rows=\d+(?:\.\d+)? loops=\d+', 'actual rows=N loops=N');
         ln := regexp_replace(ln, 'Rows Removed by Filter: \d+', 'Rows Removed by Filter: N');
         return next ln;
     end loop;
-- 
2.34.1



^ permalink  raw  reply  [nested|flat] 26+ messages in thread

* Re: explain analyze rows=%.0f
  2025-01-13 20:18 Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
  2025-02-07 19:59 ` Re: explain analyze rows=%.0f Matheus Alcantara <[email protected]>
  2025-02-07 20:41   ` Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
  2025-02-07 21:01     ` Re: explain analyze rows=%.0f Matheus Alcantara <[email protected]>
  2025-02-07 21:28       ` Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
  2025-02-11 17:14         ` Re: explain analyze rows=%.0f Andrei Lepikhov <[email protected]>
  2025-02-11 17:41           ` Re: explain analyze rows=%.0f Robert Haas <[email protected]>
  2025-02-11 20:46             ` Re: explain analyze rows=%.0f Tom Lane <[email protected]>
  2025-02-12 08:54               ` Re: explain analyze rows=%.0f Andrei Lepikhov <[email protected]>
  2025-02-12 10:10                 ` Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
  2025-02-12 18:19                   ` Re: explain analyze rows=%.0f Robert Haas <[email protected]>
  2025-02-12 18:40                     ` Re: explain analyze rows=%.0f Tom Lane <[email protected]>
  2025-02-12 19:55                       ` Re: explain analyze rows=%.0f Andrei Lepikhov <[email protected]>
  2025-02-12 19:56                         ` Re: explain analyze rows=%.0f Robert Haas <[email protected]>
  2025-02-13 09:05                           ` Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
@ 2025-02-13 20:42                             ` Robert Haas <[email protected]>
  0 siblings, 0 replies; 26+ messages in thread

From: Robert Haas @ 2025-02-13 20:42 UTC (permalink / raw)
  To: Ilia Evdokimov <[email protected]>; +Cc: [email protected]; Tom Lane <[email protected]>; Matheus Alcantara <[email protected]>; Guillaume Lelarge <[email protected]>; Daniel Gustafsson <[email protected]>; Ibrar Ahmed <[email protected]>; Gregory Stark (as CFM) <[email protected]>; Amit Kapila <[email protected]>; Justin Pryzby <[email protected]>; Peter Geoghegan <[email protected]>; vignesh C <[email protected]>; David G. Johnston <[email protected]>; Alena Rybakina <[email protected]>; Andrei Lepikhov <[email protected]>

On Thu, Feb 13, 2025 at 4:05 AM Ilia Evdokimov
<[email protected]> wrote:
> 1. Documentation (v9-0001-Clarify-display-of-rows-as-decimal-fractions-DOC.patch)
>
> One thing that bothers me is that the documentation explains how to compute total time, but it does not clarify how to compute total rows. Maybe this was obvious to others before, but now that we are displaying rows as a fraction, we should explicitly document how to interpret it alongside total time.
>
> I believe it would be helpful to show a non-integer rows value in an example query. However, to achieve this, we need the index scan results to vary across iterations. One way to accomplish this is by using the condition t1.unique2 > t2.unique2. Additionally, I suggest making loops a round number (e.g., 100) for better readability, which can be achieved using t1.thousand < 10. The final query would look like this:
>
> EXPLAIN ANALYZE SELECT *
> FROM tenk1 t1, tenk2 t2
> WHERE t1.thousand < 10 AND t1.unique2 > t2.unique2;
>
> I believe this is an ideal example for the documentation because it not only demonstrates fractional rows, but also keeps the execution plan nearly unchanged. While the estimated and actual average row counts become slightly rounded, I don't see another way to ensure different results for each index scan.

Anyone else have an opinion on this? I see Ilia's point, but a
non-equality join is probably an atypical case.

> 2. Code and tests (v9-0002-Clarify-display-of-rows-as-decimal-fractions.patch)
>
> I left the code and tests unchanged since we agreed on a fixed format of two decimal places.

This still has the HAS_DECIMAL() thing to which I objected.

-- 
Robert Haas
EDB: http://www.enterprisedb.com






^ permalink  raw  reply  [nested|flat] 26+ messages in thread


end of thread, other threads:[~2025-02-13 20:42 UTC | newest]

Thread overview: 26+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2024-03-26 13:10 [PATCH v7 11/16] Remove heap_freeze_execute_prepared() Melanie Plageman <[email protected]>
2025-01-13 20:18 Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
2025-02-07 19:59 ` Re: explain analyze rows=%.0f Matheus Alcantara <[email protected]>
2025-02-07 20:41   ` Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
2025-02-07 21:01     ` Re: explain analyze rows=%.0f Matheus Alcantara <[email protected]>
2025-02-07 21:28       ` Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
2025-02-10 15:32         ` Re: explain analyze rows=%.0f Matheus Alcantara <[email protected]>
2025-02-10 16:38           ` Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
2025-02-10 20:43             ` Re: explain analyze rows=%.0f Matheus Alcantara <[email protected]>
2025-02-10 21:14               ` Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
2025-02-11 14:51                 ` Re: explain analyze rows=%.0f Matheus Alcantara <[email protected]>
2025-02-11 17:14         ` Re: explain analyze rows=%.0f Andrei Lepikhov <[email protected]>
2025-02-11 17:41           ` Re: explain analyze rows=%.0f Robert Haas <[email protected]>
2025-02-11 19:18             ` Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
2025-02-11 19:43               ` Re: explain analyze rows=%.0f Alena Rybakina <[email protected]>
2025-02-12 18:11               ` Re: explain analyze rows=%.0f Robert Haas <[email protected]>
2025-02-11 20:19             ` Re: explain analyze rows=%.0f Andrei Lepikhov <[email protected]>
2025-02-11 20:46             ` Re: explain analyze rows=%.0f Tom Lane <[email protected]>
2025-02-12 08:54               ` Re: explain analyze rows=%.0f Andrei Lepikhov <[email protected]>
2025-02-12 10:10                 ` Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
2025-02-12 18:19                   ` Re: explain analyze rows=%.0f Robert Haas <[email protected]>
2025-02-12 18:40                     ` Re: explain analyze rows=%.0f Tom Lane <[email protected]>
2025-02-12 19:55                       ` Re: explain analyze rows=%.0f Andrei Lepikhov <[email protected]>
2025-02-12 19:56                         ` Re: explain analyze rows=%.0f Robert Haas <[email protected]>
2025-02-13 09:05                           ` Re: explain analyze rows=%.0f Ilia Evdokimov <[email protected]>
2025-02-13 20:42                             ` Re: explain analyze rows=%.0f Robert Haas <[email protected]>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox