Fix CatalogTupleInsert/Update abstraction for case of shared indstate.
authorTom Lane
Wed, 1 Feb 2017 22:18:36 +0000 (17:18 -0500)
committerTom Lane
Wed, 1 Feb 2017 22:18:36 +0000 (17:18 -0500)
Add CatalogTupleInsertWithInfo and CatalogTupleUpdateWithInfo to let
callers use the CatalogTupleXXX abstraction layer even in cases where
we want to share the results of CatalogOpenIndexes across multiple
inserts/updates for efficiency.  This finishes the job begun in commit
2f5c9d9c9, by allowing some remaining simple_heap_insert/update
calls to be replaced.  The abstraction layer is now complete enough
that we don't have to export CatalogIndexInsert at all anymore.

Also, this fixes several places in which 2f5c9d9c9 introduced performance
regressions by using retail CatalogTupleInsert or CatalogTupleUpdate even
though the previous coding had been able to amortize CatalogOpenIndexes
work across multiple tuples.

A possible future improvement is to arrange for the indexing.c functions
to cache the CatalogIndexState somewhere, maybe in the relcache, in which
case we could get rid of CatalogTupleInsertWithInfo and
CatalogTupleUpdateWithInfo again.  But that's a task for another day.

Discussion: https://postgr.es/m/27502.1485981379@sss.pgh.pa.us

src/backend/catalog/heap.c
src/backend/catalog/indexing.c
src/backend/catalog/pg_depend.c
src/backend/catalog/pg_shdepend.c
src/backend/commands/cluster.c
src/backend/storage/large_object/inv_api.c
src/include/catalog/indexing.h

index a674401e464b3e66733815709f1ae2d823815646..41c00565569546c3b3e801e4eedb471b00e6d02f 100644 (file)
@@ -586,9 +586,10 @@ CheckAttributeType(const char *attname,
  * attribute to insert (but we ignore attacl and attoptions, which are always
  * initialized to NULL).
  *
- * indstate is the index state for CatalogIndexInsert.  It can be passed as
- * NULL, in which case we'll fetch the necessary info.  (Don't do this when
- * inserting multiple attributes, because it's a tad more expensive.)
+ * indstate is the index state for CatalogTupleInsertWithInfo.  It can be
+ * passed as NULL, in which case we'll fetch the necessary info.  (Don't do
+ * this when inserting multiple attributes, because it's a tad more
+ * expensive.)
  */
 void
 InsertPgAttributeTuple(Relation pg_attribute_rel,
@@ -630,18 +631,10 @@ InsertPgAttributeTuple(Relation pg_attribute_rel,
    tup = heap_form_tuple(RelationGetDescr(pg_attribute_rel), values, nulls);
 
    /* finally insert the new tuple, update the indexes, and clean up */
-   simple_heap_insert(pg_attribute_rel, tup);
-
    if (indstate != NULL)
-       CatalogIndexInsert(indstate, tup);
+       CatalogTupleInsertWithInfo(pg_attribute_rel, tup, indstate);
    else
-   {
-       CatalogIndexState indstate;
-
-       indstate = CatalogOpenIndexes(pg_attribute_rel);
-       CatalogIndexInsert(indstate, tup);
-       CatalogCloseIndexes(indstate);
-   }
+       CatalogTupleInsert(pg_attribute_rel, tup);
 
    heap_freetuple(tup);
 }
index 02f64813ec54cb2fd3460c8ee8e6d8faaf2c3ed0..76268e1d2a27a5898bfc5038494d555defbf91ea 100644 (file)
@@ -68,7 +68,7 @@ CatalogCloseIndexes(CatalogIndexState indstate)
  *
  * This is effectively a cut-down version of ExecInsertIndexTuples.
  */
-void
+static void
 CatalogIndexInsert(CatalogIndexState indstate, HeapTuple heapTuple)
 {
    int         i;
@@ -148,18 +148,20 @@ CatalogIndexInsert(CatalogIndexState indstate, HeapTuple heapTuple)
 /*
  * CatalogTupleInsert - do heap and indexing work for a new catalog tuple
  *
+ * Insert the tuple data in "tup" into the specified catalog relation.
+ * The Oid of the inserted tuple is returned.
+ *
  * This is a convenience routine for the common case of inserting a single
  * tuple in a system catalog; it inserts a new heap tuple, keeping indexes
- * current.  Avoid using it for multiple tuples, since opening the indexes and
- * building the index info structures is moderately expensive.
- *
- * The Oid of the inserted tuple is returned.
+ * current.  Avoid using it for multiple tuples, since opening the indexes
+ * and building the index info structures is moderately expensive.
+ * (Use CatalogTupleInsertWithInfo in such cases.)
  */
 Oid
 CatalogTupleInsert(Relation heapRel, HeapTuple tup)
 {
    CatalogIndexState indstate;
-   Oid     oid;
+   Oid         oid;
 
    indstate = CatalogOpenIndexes(heapRel);
 
@@ -171,14 +173,37 @@ CatalogTupleInsert(Relation heapRel, HeapTuple tup)
    return oid;
 }
 
+/*
+ * CatalogTupleInsertWithInfo - as above, but with caller-supplied index info
+ *
+ * This should be used when it's important to amortize CatalogOpenIndexes/
+ * CatalogCloseIndexes work across multiple insertions.  At some point we
+ * might cache the CatalogIndexState data somewhere (perhaps in the relcache)
+ * so that callers needn't trouble over this ... but we don't do so today.
+ */
+Oid
+CatalogTupleInsertWithInfo(Relation heapRel, HeapTuple tup,
+                          CatalogIndexState indstate)
+{
+   Oid         oid;
+
+   oid = simple_heap_insert(heapRel, tup);
+
+   CatalogIndexInsert(indstate, tup);
+
+   return oid;
+}
+
 /*
  * CatalogTupleUpdate - do heap and indexing work for updating a catalog tuple
  *
+ * Update the tuple identified by "otid", replacing it with the data in "tup".
+ *
  * This is a convenience routine for the common case of updating a single
- * tuple in a system catalog; it updates one heap tuple (identified by otid)
- * with tup, keeping indexes current.  Avoid using it for multiple tuples,
- * since opening the indexes and building the index info structures is
- * moderately expensive.
+ * tuple in a system catalog; it updates one heap tuple, keeping indexes
+ * current.  Avoid using it for multiple tuples, since opening the indexes
+ * and building the index info structures is moderately expensive.
+ * (Use CatalogTupleUpdateWithInfo in such cases.)
  */
 void
 CatalogTupleUpdate(Relation heapRel, ItemPointer otid, HeapTuple tup)
@@ -193,15 +218,37 @@ CatalogTupleUpdate(Relation heapRel, ItemPointer otid, HeapTuple tup)
    CatalogCloseIndexes(indstate);
 }
 
+/*
+ * CatalogTupleUpdateWithInfo - as above, but with caller-supplied index info
+ *
+ * This should be used when it's important to amortize CatalogOpenIndexes/
+ * CatalogCloseIndexes work across multiple updates.  At some point we
+ * might cache the CatalogIndexState data somewhere (perhaps in the relcache)
+ * so that callers needn't trouble over this ... but we don't do so today.
+ */
+void
+CatalogTupleUpdateWithInfo(Relation heapRel, ItemPointer otid, HeapTuple tup,
+                          CatalogIndexState indstate)
+{
+   simple_heap_update(heapRel, otid, tup);
+
+   CatalogIndexInsert(indstate, tup);
+}
+
 /*
  * CatalogTupleDelete - do heap and indexing work for deleting a catalog tuple
  *
- * Delete the tuple identified by tid in the specified catalog.
+ * Delete the tuple identified by "tid" in the specified catalog.
  *
  * With Postgres heaps, there is no index work to do at deletion time;
  * cleanup will be done later by VACUUM.  However, callers of this function
  * shouldn't have to know that; we'd like a uniform abstraction for all
  * catalog tuple changes.  Hence, provide this currently-trivial wrapper.
+ *
+ * The abstraction is a bit leaky in that we don't provide an optimized
+ * CatalogTupleDeleteWithInfo version, because there is currently nothing to
+ * optimize.  If we ever need that, rather than touching a lot of call sites,
+ * it might be better to do something about caching CatalogIndexState.
  */
 void
 CatalogTupleDelete(Relation heapRel, ItemPointer tid)
index 722df67bda038f64d68ecbb7299c2eccefe7e557..d0ee851215d1477cc88d25b34219b87993945f84 100644 (file)
@@ -107,13 +107,11 @@ recordMultipleDependencies(const ObjectAddress *depender,
 
            tup = heap_form_tuple(dependDesc->rd_att, values, nulls);
 
-           simple_heap_insert(dependDesc, tup);
-
-           /* keep indexes current */
+           /* fetch index info only when we know we need it */
            if (indstate == NULL)
                indstate = CatalogOpenIndexes(dependDesc);
 
-           CatalogIndexInsert(indstate, tup);
+           CatalogTupleInsertWithInfo(dependDesc, tup, indstate);
 
            heap_freetuple(tup);
        }
index ef50b854d4d9d00081abfef2d7e2cb1e6d9f7d03..8d946ff44ccb341172fe4fa562a49ec46382f46a 100644 (file)
@@ -753,7 +753,7 @@ copyTemplateDependencies(Oid templateDbId, Oid newDbId)
        HeapTuple   newtup;
 
        newtup = heap_modify_tuple(tup, sdepDesc, values, nulls, replace);
-       CatalogTupleInsert(sdepRel, newtup);
+       CatalogTupleInsertWithInfo(sdepRel, newtup, indstate);
 
        heap_freetuple(newtup);
    }
index e60e6133155aad81300f26a73df17c92305a8bc9..330385ba0a69183007187c4f517f18a9bf294724 100644 (file)
@@ -1141,7 +1141,6 @@ swap_relation_files(Oid r1, Oid r2, bool target_is_pg_class,
                relfilenode2;
    Oid         swaptemp;
    char        swptmpchr;
-   CatalogIndexState indstate;
 
    /* We need writable copies of both pg_class tuples. */
    relRelation = heap_open(RelationRelationId, RowExclusiveLock);
@@ -1285,13 +1284,13 @@ swap_relation_files(Oid r1, Oid r2, bool target_is_pg_class,
     */
    if (!target_is_pg_class)
    {
-       simple_heap_update(relRelation, &reltup1->t_self, reltup1);
-       simple_heap_update(relRelation, &reltup2->t_self, reltup2);
+       CatalogIndexState indstate;
 
-       /* Keep system catalogs current */
        indstate = CatalogOpenIndexes(relRelation);
-       CatalogIndexInsert(indstate, reltup1);
-       CatalogIndexInsert(indstate, reltup2);
+       CatalogTupleUpdateWithInfo(relRelation, &reltup1->t_self, reltup1,
+                                  indstate);
+       CatalogTupleUpdateWithInfo(relRelation, &reltup2->t_self, reltup2,
+                                  indstate);
        CatalogCloseIndexes(indstate);
    }
    else
index 8eb5b3b08e36830ac48cfeaedc8c167756b3ba56..6597b36b1725ebda8a7ef8bd50638fdd8cc67729 100644 (file)
@@ -678,7 +678,8 @@ inv_write(LargeObjectDesc *obj_desc, const char *buf, int nbytes)
            replace[Anum_pg_largeobject_data - 1] = true;
            newtup = heap_modify_tuple(oldtuple, RelationGetDescr(lo_heap_r),
                                       values, nulls, replace);
-           CatalogTupleUpdate(lo_heap_r, &newtup->t_self, newtup);
+           CatalogTupleUpdateWithInfo(lo_heap_r, &newtup->t_self, newtup,
+                                      indstate);
            heap_freetuple(newtup);
 
            /*
@@ -720,7 +721,7 @@ inv_write(LargeObjectDesc *obj_desc, const char *buf, int nbytes)
            values[Anum_pg_largeobject_pageno - 1] = Int32GetDatum(pageno);
            values[Anum_pg_largeobject_data - 1] = PointerGetDatum(&workbuf);
            newtup = heap_form_tuple(lo_heap_r->rd_att, values, nulls);
-           CatalogTupleInsert(lo_heap_r, newtup);
+           CatalogTupleInsertWithInfo(lo_heap_r, newtup, indstate);
            heap_freetuple(newtup);
        }
        pageno++;
@@ -848,7 +849,8 @@ inv_truncate(LargeObjectDesc *obj_desc, int64 len)
        replace[Anum_pg_largeobject_data - 1] = true;
        newtup = heap_modify_tuple(oldtuple, RelationGetDescr(lo_heap_r),
                                   values, nulls, replace);
-       CatalogTupleUpdate(lo_heap_r, &newtup->t_self, newtup);
+       CatalogTupleUpdateWithInfo(lo_heap_r, &newtup->t_self, newtup,
+                                  indstate);
        heap_freetuple(newtup);
    }
    else
@@ -885,7 +887,7 @@ inv_truncate(LargeObjectDesc *obj_desc, int64 len)
        values[Anum_pg_largeobject_pageno - 1] = Int32GetDatum(pageno);
        values[Anum_pg_largeobject_data - 1] = PointerGetDatum(&workbuf);
        newtup = heap_form_tuple(lo_heap_r->rd_att, values, nulls);
-       CatalogTupleInsert(lo_heap_r, newtup);
+       CatalogTupleInsertWithInfo(lo_heap_r, newtup, indstate);
        heap_freetuple(newtup);
    }
 
index 9d02666ed1bf4c9a624375799ef3105145532ac6..6bce7328a289f1f6b6d949186dde616912ee540f 100644 (file)
@@ -30,11 +30,14 @@ typedef struct ResultRelInfo *CatalogIndexState;
  */
 extern CatalogIndexState CatalogOpenIndexes(Relation heapRel);
 extern void CatalogCloseIndexes(CatalogIndexState indstate);
-extern void CatalogIndexInsert(CatalogIndexState indstate,
-                  HeapTuple heapTuple);
-extern Oid CatalogTupleInsert(Relation heapRel, HeapTuple tup);
+extern Oid CatalogTupleInsert(Relation heapRel, HeapTuple tup);
+extern Oid CatalogTupleInsertWithInfo(Relation heapRel, HeapTuple tup,
+                          CatalogIndexState indstate);
 extern void CatalogTupleUpdate(Relation heapRel, ItemPointer otid,
                   HeapTuple tup);
+extern void CatalogTupleUpdateWithInfo(Relation heapRel,
+                          ItemPointer otid, HeapTuple tup,
+                          CatalogIndexState indstate);
 extern void CatalogTupleDelete(Relation heapRel, ItemPointer tid);