Fix IndexOnlyScan counter for heap fetches in parallel mode

The HeapFetches counter was using a simple value in IndexOnlyScanState,
which fails to propagate values from parallel workers; so the counts are
wrong when IndexOnlyScan runs in parallel.  Move it to Instrumentation,
like all the other counters.

While at it, change INSERT ON CONFLICT conflicting tuple counter to use
the new ntuples2 instead of nfiltered2, which is a blatant misuse.

Discussion: https://postgr.es/m/20180409215851.idwc75ct2bzi6tea@alvherre.pgsql

diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index 989b6aad67bc75592b9bc7d822aae2a7809ba9db..306e6444669604fc537b717aa8da1e53659166dd 100644 (file)
--- 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);
-           }
+               ExplainPropertyFloat("Heap Fetches", NULL,
+                                    planstate->instrument->ntuples2, 0, es);
            break;
        case T_BitmapIndexScan:
            show_scan_qual(((BitmapIndexScan *) plan)->indexqualorig,
@@ -3132,7 +3128,7 @@ show_modifytable_info(ModifyTableState *mtstate, List *ancestors,
 
            /* count the number of source rows */
            total = mtstate->mt_plans[0]->instrument->ntuples;
-           other_path = mtstate->ps.instrument->nfiltered2;
+           other_path = mtstate->ps.instrument->ntuples2;
            insert_path = total - other_path;
 
            ExplainPropertyFloat("Tuples Inserted", NULL,

diff --git a/src/backend/executor/instrument.c b/src/backend/executor/instrument.c
index 86252cee1f31b462058cc69c108fb1bad234d79c..fe5d55904d27f2f2bc5f77b013d5c52a32d5b5d2 100644 (file)
--- a/src/backend/executor/instrument.c
+++ b/src/backend/executor/instrument.c
@@ -156,6 +156,7 @@ InstrAggNode(Instrumentation *dst, Instrumentation *add)
    dst->startup += add->startup;
    dst->total += add->total;
    dst->ntuples += add->ntuples;
+   dst->ntuples2 += add->ntuples2;
    dst->nloops += add->nloops;
    dst->nfiltered1 += add->nfiltered1;
    dst->nfiltered2 += add->nfiltered2;

diff --git a/src/backend/executor/nodeIndexonlyscan.c b/src/backend/executor/nodeIndexonlyscan.c
index ddc0ae90615f9f10f2a563314572a92043786eba..3a02a996214985f6c067407b50fb1419988117fd 100644 (file)
--- 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++;
+           InstrCountTuples2(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/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index f47649d0517555e028470cf5b3e41af0f8286186..543a735be2b81e664339d6c402b6027f4c260e27 100644 (file)
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -461,7 +461,7 @@ ExecInsert(ModifyTableState *mtstate,
                                             &conflictTid, planSlot, slot,
                                             estate, canSetTag, &returning))
                    {
-                       InstrCountFiltered2(&mtstate->ps, 1);
+                       InstrCountTuples2(&mtstate->ps, 1);
                        return returning;
                    }
                    else
@@ -476,7 +476,7 @@ ExecInsert(ModifyTableState *mtstate,
                     */
                    Assert(onconflict == ONCONFLICT_NOTHING);
                    ExecCheckTIDVisible(estate, resultRelInfo, &conflictTid);
-                   InstrCountFiltered2(&mtstate->ps, 1);
+                   InstrCountTuples2(&mtstate->ps, 1);
                    return NULL;
                }
            }

diff --git a/src/include/executor/instrument.h b/src/include/executor/instrument.h
index 28eb0093d47ae0467a919a726b0b3253f0d4d1a0..1bc7a88dbd682441b110b9304e1009c86c348b53 100644 (file)
--- a/src/include/executor/instrument.h
+++ b/src/include/executor/instrument.h
@@ -57,6 +57,7 @@ typedef struct Instrumentation
    double      startup;        /* Total startup time (in seconds) */
    double      total;          /* Total total time (in seconds) */
    double      ntuples;        /* Total tuples produced */
+   double      ntuples2;       /* Secondary node-specific tuple counter */
    double      nloops;         /* # of run cycles for this node */
    double      nfiltered1;     /* # tuples removed by scanqual or joinqual OR
                                 * # tuples inserted by MERGE */

diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index 06456f07cc79eaf92a24922df70bd0e4ee200200..deab87546635e228a0d41172fb5eeecbe527bf81 100644 (file)
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -1004,6 +1004,11 @@ typedef struct PlanState
 #define outerPlanState(node)       (((PlanState *)(node))->lefttree)
 
 /* Macros for inline access to certain instrumentation counters */
+#define InstrCountTuples2(node, delta) \
+   do { \
+       if (((PlanState *)(node))->instrument) \
+           ((PlanState *)(node))->instrument->ntuples2 += (delta); \
+   } while (0)
 #define InstrCountFiltered1(node, delta) \
    do { \
        if (((PlanState *)(node))->instrument) \
@@ -1368,7 +1373,6 @@ typedef struct IndexScanState
  *     RelationDesc       index relation descriptor
  *     ScanDesc           index scan descriptor
  *     VMBuffer           buffer in use for visibility map testing, if any
- *     HeapFetches        number of tuples we were forced to fetch from heap
  *     ioss_PscanLen      Size of parallel index-only scan descriptor
  * ----------------
  */
@@ -1387,7 +1391,6 @@ typedef struct IndexOnlyScanState
    Relation    ioss_RelationDesc;
    IndexScanDesc ioss_ScanDesc;
    Buffer      ioss_VMBuffer;
-   long        ioss_HeapFetches;
    Size        ioss_PscanLen;
 } IndexOnlyScanState;