Andrew Gierth wrote:
> >>>>> "Alvaro" == Alvaro Herrera <alvhe...@alvh.no-ip.org> writes:
>
> Alvaro> Thanks for cleaning that up. I'll look into why the test
> Alvaro> (without this commit) fails with force_parallel_mode=regress
> Alvaro> next week.
>
> Seems clear enough to me - the "Heap Fetches" statistic is kept in the
> IndexOnlyScanState node in its own field, not part of ss.ps.instrument,
> and is therefore not reported from workers to leader.
Right, thanks for the pointer.
So here's a patch that makes thing behave as expected. I noticed that
instrument->nfiltered3 was available, so I used that to keep the
counter. I wanted to print it using show_instrumentation_count (which
has the nice property that you don't even have to test for es->analyze),
but it doesn't work, because it divides the number by nloops, which is
not what we want in this case. (It also doesn't print if the counter is
zero, which maybe is desirable for the other counters but probably not
for this one).
I then noticed that support for nfiltered3 was incomplete; hence 0001.
(I then noticed that nfiltered3 was added for MERGE. It looks wrong to
me.)
Frankly, I don't like this. I would rather have an instrument->ntuples2
rather than these "divide this by nloops, sometimes" schizoid counters.
This is already being misused by ON CONFLICT (see "other_path" in
show_modifytable_info). But it seems like a correct fix would require
more code.
Anyway, the partition_prune test works correctly now (after reverting
AndrewSN's b47a86f5008f26) in both force_parallel_mode settings.
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>From 10b29c7706efd00279182164de14592643ff7f40 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvhe...@alvh.no-ip.org>
Date: Mon, 9 Apr 2018 18:42:04 -0300
Subject: [PATCH 1/2] print nfiltered3
---
src/backend/commands/explain.c | 4 +++-
src/backend/executor/instrument.c | 1 +
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index 989b6aad67..347fbbe1cf 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -2592,7 +2592,9 @@ show_instrumentation_count(const char *qlabel, int which,
if (!es->analyze || !planstate->instrument)
return;
- if (which == 2)
+ if (which == 3)
+ nfiltered = planstate->instrument->nfiltered3;
+ else if (which == 2)
nfiltered = planstate->instrument->nfiltered2;
else
nfiltered = planstate->instrument->nfiltered1;
diff --git a/src/backend/executor/instrument.c
b/src/backend/executor/instrument.c
index 86252cee1f..d3045f57ac 100644
--- a/src/backend/executor/instrument.c
+++ b/src/backend/executor/instrument.c
@@ -159,6 +159,7 @@ InstrAggNode(Instrumentation *dst, Instrumentation *add)
dst->nloops += add->nloops;
dst->nfiltered1 += add->nfiltered1;
dst->nfiltered2 += add->nfiltered2;
+ dst->nfiltered3 += add->nfiltered3;
/* Add delta of buffer usage since entry to node's totals */
if (dst->need_bufusage)
--
2.11.0
>From acaf8e8af643f05605eab21ad3e5a04a8714f06a Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvhe...@alvh.no-ip.org>
Date: Mon, 9 Apr 2018 18:41:20 -0300
Subject: [PATCH 2/2] use nfiltered3 instead of ad-hoc counter
---
src/backend/commands/explain.c | 8 ++------
src/backend/executor/nodeIndexonlyscan.c | 3 +--
src/include/nodes/execnodes.h | 1 -
3 files changed, 3 insertions(+), 9 deletions(-)
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index 347fbbe1cf..434051e7df 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -1459,12 +1459,8 @@ ExplainNode(PlanState *planstate, List *ancestors,
show_instrumentation_count("Rows Removed by
Filter", 1,
planstate, es);
if (es->analyze)
- {
- long heapFetches =
- ((IndexOnlyScanState *)
planstate)->ioss_HeapFetches;
-
- ExplainPropertyInteger("Heap Fetches", NULL,
heapFetches, es);
- }
+ ExplainPropertyInteger("Heap Fetches", NULL,
+
planstate->instrument->nfiltered3, es);
break;
case T_BitmapIndexScan:
show_scan_qual(((BitmapIndexScan *)
plan)->indexqualorig,
diff --git a/src/backend/executor/nodeIndexonlyscan.c
b/src/backend/executor/nodeIndexonlyscan.c
index ddc0ae9061..ef835f13c2 100644
--- a/src/backend/executor/nodeIndexonlyscan.c
+++ b/src/backend/executor/nodeIndexonlyscan.c
@@ -162,7 +162,7 @@ IndexOnlyNext(IndexOnlyScanState *node)
/*
* Rats, we have to visit the heap to check visibility.
*/
- node->ioss_HeapFetches++;
+ InstrCountFiltered3(node, 1);
tuple = index_fetch_heap(scandesc);
if (tuple == NULL)
continue; /* no visible tuple,
try next index entry */
@@ -509,7 +509,6 @@ ExecInitIndexOnlyScan(IndexOnlyScan *node, EState *estate,
int eflags)
indexstate->ss.ps.plan = (Plan *) node;
indexstate->ss.ps.state = estate;
indexstate->ss.ps.ExecProcNode = ExecIndexOnlyScan;
- indexstate->ioss_HeapFetches = 0;
/*
* Miscellaneous initialization
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index 06456f07cc..d00839c161 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -1387,7 +1387,6 @@ typedef struct IndexOnlyScanState
Relation ioss_RelationDesc;
IndexScanDesc ioss_ScanDesc;
Buffer ioss_VMBuffer;
- long ioss_HeapFetches;
Size ioss_PscanLen;
} IndexOnlyScanState;
--
2.11.0
