Allow index AMs to return either HeapTuple or IndexTuple format during IOS.
authorTom Lane <[email protected]>
Mon, 27 Feb 2017 22:20:34 +0000 (17:20 -0500)
committerTom Lane <[email protected]>
Mon, 27 Feb 2017 22:20:34 +0000 (17:20 -0500)
Previously, only IndexTuple format was supported for the output data of
an index-only scan.  This is fine for btree, which is just returning a
verbatim index tuple anyway.  It's not so fine for SP-GiST, which can
return reconstructed data that's much larger than a page.

To fix, extend the index AM API so that index-only scan data can be
returned in either HeapTuple or IndexTuple format.  There's other ways
we could have done it, but this way avoids an API break for index AMs
that aren't concerned with the issue, and it costs little except a couple
more fields in IndexScanDescs.

I changed both GiST and SP-GiST to use the HeapTuple method.  I'm not
very clear on whether GiST can reconstruct data that's too large for an
IndexTuple, but that seems possible, and it's not much of a code change to
fix.

Per a complaint from Vik Fearing.  Reviewed by Jason Li.

Discussion: https://postgr.es/m/49527f79-530d-0bfe-3dad-d183596afa92@2ndquadrant.fr

doc/src/sgml/indexam.sgml
src/backend/access/gist/gistget.c
src/backend/access/gist/gistscan.c
src/backend/access/gist/gistutil.c
src/backend/access/index/genam.c
src/backend/access/index/indexam.c
src/backend/access/spgist/spgscan.c
src/backend/executor/nodeIndexonlyscan.c
src/include/access/gist_private.h
src/include/access/relscan.h
src/include/access/spgist_private.h

index 401b11598eb37528a1a87c19222bacbf0305f714..ac512588e24ce904414d8ae490a0a92a41b06f0c 100644 (file)
@@ -551,15 +551,19 @@ amgettuple (IndexScanDesc scan,
   <para>
    If the index supports <link linkend="indexes-index-only-scans">index-only
    scans</link> (i.e., <function>amcanreturn</function> returns TRUE for it),
-   then on success the AM must also check
-   <literal>scan-&gt;xs_want_itup</>, and if that is true it must return
-   the original indexed data for the index entry, in the form of an
+   then on success the AM must also check <literal>scan-&gt;xs_want_itup</>,
+   and if that is true it must return the originally indexed data for the
+   index entry.  The data can be returned in the form of an
    <structname>IndexTuple</> pointer stored at <literal>scan-&gt;xs_itup</>,
-   with tuple descriptor <literal>scan-&gt;xs_itupdesc</>.
-   (Management of the data referenced by the pointer is the access method's
+   with tuple descriptor <literal>scan-&gt;xs_itupdesc</>; or in the form of
+   a <structname>HeapTuple</> pointer stored at <literal>scan-&gt;xs_hitup</>,
+   with tuple descriptor <literal>scan-&gt;xs_hitupdesc</>.  (The latter
+   format should be used when reconstructing data that might possibly not fit
+   into an IndexTuple.)  In either case,
+   management of the data referenced by the pointer is the access method's
    responsibility.  The data must remain good at least until the next
    <function>amgettuple</>, <function>amrescan</>, or <function>amendscan</>
-   call for the scan.)
+   call for the scan.
   </para>
 
   <para>
index eea366b1ad43505eef9e24917c0008d03aa77089..122dc38db565ca643eaa71cc6ab622d9a751e6db 100644 (file)
@@ -441,12 +441,13 @@ gistScanPage(IndexScanDesc scan, GISTSearchItem *pageItem, double *myDistances,
            so->pageData[so->nPageData].offnum = i;
 
            /*
-            * In an index-only scan, also fetch the data from the tuple.
+            * In an index-only scan, also fetch the data from the tuple.  The
+            * reconstructed tuples are stored in pageDataCxt.
             */
            if (scan->xs_want_itup)
            {
                oldcxt = MemoryContextSwitchTo(so->pageDataCxt);
-               so->pageData[so->nPageData].ftup =
+               so->pageData[so->nPageData].recontup =
                    gistFetchTuple(giststate, r, it);
                MemoryContextSwitchTo(oldcxt);
            }
@@ -478,7 +479,7 @@ gistScanPage(IndexScanDesc scan, GISTSearchItem *pageItem, double *myDistances,
                 * In an index-only scan, also fetch the data from the tuple.
                 */
                if (scan->xs_want_itup)
-                   item->data.heap.ftup = gistFetchTuple(giststate, r, it);
+                   item->data.heap.recontup = gistFetchTuple(giststate, r, it);
            }
            else
            {
@@ -540,11 +541,11 @@ getNextNearest(IndexScanDesc scan)
    bool        res = false;
    int         i;
 
-   if (scan->xs_itup)
+   if (scan->xs_hitup)
    {
        /* free previously returned tuple */
-       pfree(scan->xs_itup);
-       scan->xs_itup = NULL;
+       pfree(scan->xs_hitup);
+       scan->xs_hitup = NULL;
    }
 
    do
@@ -601,7 +602,7 @@ getNextNearest(IndexScanDesc scan)
 
            /* in an index-only scan, also return the reconstructed tuple. */
            if (scan->xs_want_itup)
-               scan->xs_itup = item->data.heap.ftup;
+               scan->xs_hitup = item->data.heap.recontup;
            res = true;
        }
        else
@@ -685,7 +686,7 @@ gistgettuple(IndexScanDesc scan, ScanDirection dir)
 
                /* in an index-only scan, also return the reconstructed tuple */
                if (scan->xs_want_itup)
-                   scan->xs_itup = so->pageData[so->curPageData].ftup;
+                   scan->xs_hitup = so->pageData[so->curPageData].recontup;
 
                so->curPageData++;
 
index 33b388906aa6652080dcf1d7d05758019349c275..81ff8fc8b6e1e3095c88ff44a3824413eccfa60d 100644 (file)
@@ -155,7 +155,7 @@ gistrescan(IndexScanDesc scan, ScanKey key, int nkeys,
     * tuple descriptor to represent the returned index tuples and create a
     * memory context to hold them during the scan.
     */
-   if (scan->xs_want_itup && !scan->xs_itupdesc)
+   if (scan->xs_want_itup && !scan->xs_hitupdesc)
    {
        int         natts;
        int         attno;
@@ -174,8 +174,9 @@ gistrescan(IndexScanDesc scan, ScanKey key, int nkeys,
                               scan->indexRelation->rd_opcintype[attno - 1],
                               -1, 0);
        }
-       scan->xs_itupdesc = so->giststate->fetchTupdesc;
+       scan->xs_hitupdesc = so->giststate->fetchTupdesc;
 
+       /* Also create a memory context that will hold the returned tuples */
        so->pageDataCxt = AllocSetContextCreate(so->giststate->scanCxt,
                                                "GiST page data context",
                                                ALLOCSET_DEFAULT_SIZES);
index f92baedffdd3a149c5e0c73f851936a9b1433edd..75845ba0e761d531ad1bd1d5fa391560aa5aa252 100644 (file)
@@ -624,9 +624,9 @@ gistFetchAtt(GISTSTATE *giststate, int nkey, Datum k, Relation r)
 
 /*
  * Fetch all keys in tuple.
- * returns new IndexTuple that contains GISTENTRY with fetched data
+ * Returns a new HeapTuple containing the originally-indexed data.
  */
-IndexTuple
+HeapTuple
 gistFetchTuple(GISTSTATE *giststate, Relation r, IndexTuple tuple)
 {
    MemoryContext oldcxt = MemoryContextSwitchTo(giststate->tempCxt);
@@ -660,7 +660,7 @@ gistFetchTuple(GISTSTATE *giststate, Relation r, IndexTuple tuple)
    }
    MemoryContextSwitchTo(oldcxt);
 
-   return index_form_tuple(giststate->fetchTupdesc, fetchatt, isnull);
+   return heap_form_tuple(giststate->fetchTupdesc, fetchatt, isnull);
 }
 
 float
index c4a393f34e81a224d51b196c31e1e5adbb6e9733..35994769307b0d122e476033b02ca66d3a750459 100644 (file)
@@ -119,6 +119,8 @@ RelationGetIndexScan(Relation indexRelation, int nkeys, int norderbys)
 
    scan->xs_itup = NULL;
    scan->xs_itupdesc = NULL;
+   scan->xs_hitup = NULL;
+   scan->xs_hitupdesc = NULL;
 
    ItemPointerSetInvalid(&scan->xs_ctup.t_self);
    scan->xs_ctup.t_data = NULL;
index 4e7eca73ccef51e1bed2c8b1f3abd449a7510b84..cc5ac8b8571fa95a698f37690b68d8a5f6155b69 100644 (file)
@@ -535,8 +535,8 @@ index_getnext_tid(IndexScanDesc scan, ScanDirection direction)
    /*
     * The AM's amgettuple proc finds the next index entry matching the scan
     * keys, and puts the TID into scan->xs_ctup.t_self.  It should also set
-    * scan->xs_recheck and possibly scan->xs_itup, though we pay no attention
-    * to those fields here.
+    * scan->xs_recheck and possibly scan->xs_itup/scan->xs_hitup, though we
+    * pay no attention to those fields here.
     */
    found = scan->indexRelation->rd_amroutine->amgettuple(scan, direction);
 
index 139d9986004403325fd4f56e7af841ab08837813..2d96c0094e6ffabfd2da87030e40d3bd9531d866 100644 (file)
@@ -92,11 +92,11 @@ resetSpGistScanOpaque(SpGistScanOpaque so)
 
    if (so->want_itup)
    {
-       /* Must pfree IndexTuples to avoid memory leak */
+       /* Must pfree reconstructed tuples to avoid memory leak */
        int         i;
 
        for (i = 0; i < so->nPtrs; i++)
-           pfree(so->indexTups[i]);
+           pfree(so->reconTups[i]);
    }
    so->iPtr = so->nPtrs = 0;
 }
@@ -195,8 +195,8 @@ spgbeginscan(Relation rel, int keysz, int orderbysz)
                                        "SP-GiST search temporary context",
                                        ALLOCSET_DEFAULT_SIZES);
 
-   /* Set up indexTupDesc and xs_itupdesc in case it's an index-only scan */
-   so->indexTupDesc = scan->xs_itupdesc = RelationGetDescr(rel);
+   /* Set up indexTupDesc and xs_hitupdesc in case it's an index-only scan */
+   so->indexTupDesc = scan->xs_hitupdesc = RelationGetDescr(rel);
 
    scan->opaque = so;
 
@@ -591,12 +591,12 @@ storeGettuple(SpGistScanOpaque so, ItemPointer heapPtr,
    if (so->want_itup)
    {
        /*
-        * Reconstruct desired IndexTuple.  We have to copy the datum out of
-        * the temp context anyway, so we may as well create the tuple here.
+        * Reconstruct index data.  We have to copy the datum out of the temp
+        * context anyway, so we may as well create the tuple here.
         */
-       so->indexTups[so->nPtrs] = index_form_tuple(so->indexTupDesc,
-                                                   &leafValue,
-                                                   &isnull);
+       so->reconTups[so->nPtrs] = heap_form_tuple(so->indexTupDesc,
+                                                  &leafValue,
+                                                  &isnull);
    }
    so->nPtrs++;
 }
@@ -619,18 +619,18 @@ spggettuple(IndexScanDesc scan, ScanDirection dir)
            /* continuing to return tuples from a leaf page */
            scan->xs_ctup.t_self = so->heapPtrs[so->iPtr];
            scan->xs_recheck = so->recheck[so->iPtr];
-           scan->xs_itup = so->indexTups[so->iPtr];
+           scan->xs_hitup = so->reconTups[so->iPtr];
            so->iPtr++;
            return true;
        }
 
        if (so->want_itup)
        {
-           /* Must pfree IndexTuples to avoid memory leak */
+           /* Must pfree reconstructed tuples to avoid memory leak */
            int         i;
 
            for (i = 0; i < so->nPtrs; i++)
-               pfree(so->indexTups[i]);
+               pfree(so->reconTups[i]);
        }
        so->iPtr = so->nPtrs = 0;
 
index 66c2ad66d719e5f648b6a5a64e1b941056b7cc5d..4a7f39a7c7d096b1fe3547f7ef306cbdc5841396 100644 (file)
@@ -149,9 +149,26 @@ IndexOnlyNext(IndexOnlyScanState *node)
        }
 
        /*
-        * Fill the scan tuple slot with data from the index.
+        * Fill the scan tuple slot with data from the index.  This might be
+        * provided in either HeapTuple or IndexTuple format.  Conceivably an
+        * index AM might fill both fields, in which case we prefer the heap
+        * format, since it's probably a bit cheaper to fill a slot from.
         */
-       StoreIndexTuple(slot, scandesc->xs_itup, scandesc->xs_itupdesc);
+       if (scandesc->xs_hitup)
+       {
+           /*
+            * We don't take the trouble to verify that the provided tuple has
+            * exactly the slot's format, but it seems worth doing a quick
+            * check on the number of fields.
+            */
+           Assert(slot->tts_tupleDescriptor->natts ==
+                  scandesc->xs_hitupdesc->natts);
+           ExecStoreTuple(scandesc->xs_hitup, slot, InvalidBuffer, false);
+       }
+       else if (scandesc->xs_itup)
+           StoreIndexTuple(slot, scandesc->xs_itup, scandesc->xs_itupdesc);
+       else
+           elog(ERROR, "no data returned for index-only scan");
 
        /*
         * If the index was lossy, we have to recheck the index quals.
index 5b3303056b01003355df1c37134496604d08feea..1ad4ed6da7518ff07ad6ee83e0636cf356a17e6f 100644 (file)
@@ -119,7 +119,7 @@ typedef struct GISTSearchHeapItem
    ItemPointerData heapPtr;
    bool        recheck;        /* T if quals must be rechecked */
    bool        recheckDistances;       /* T if distances must be rechecked */
-   IndexTuple  ftup;           /* data fetched back from the index, used in
+   HeapTuple   recontup;       /* data reconstructed from the index, used in
                                 * index-only scans */
    OffsetNumber offnum;        /* track offset in page to mark tuple as
                                 * LP_DEAD */
@@ -477,7 +477,7 @@ extern void gistMakeUnionItVec(GISTSTATE *giststate, IndexTuple *itvec, int len,
 extern bool gistKeyIsEQ(GISTSTATE *giststate, int attno, Datum a, Datum b);
 extern void gistDeCompressAtt(GISTSTATE *giststate, Relation r, IndexTuple tuple, Page p,
                  OffsetNumber o, GISTENTRY *attdata, bool *isnull);
-extern IndexTuple gistFetchTuple(GISTSTATE *giststate, Relation r,
+extern HeapTuple gistFetchTuple(GISTSTATE *giststate, Relation r,
               IndexTuple tuple);
 extern void gistMakeUnionKey(GISTSTATE *giststate, int attno,
                 GISTENTRY *entry1, bool isnull1,
index ce3ca8d4ac27bf574f0376577e568a037902a693..3fc726d712f64ee7979b51b6d0714e410230f23b 100644 (file)
@@ -104,9 +104,16 @@ typedef struct IndexScanDescData
    /* index access method's private state */
    void       *opaque;         /* access-method-specific info */
 
-   /* in an index-only scan, this is valid after a successful amgettuple */
+   /*
+    * In an index-only scan, a successful amgettuple call must fill either
+    * xs_itup (and xs_itupdesc) or xs_hitup (and xs_hitupdesc) to provide the
+    * data returned by the scan.  It can fill both, in which case the heap
+    * format will be used.
+    */
    IndexTuple  xs_itup;        /* index tuple returned by AM */
    TupleDesc   xs_itupdesc;    /* rowtype descriptor of xs_itup */
+   HeapTuple   xs_hitup;       /* index data returned by AM, as HeapTuple */
+   TupleDesc   xs_hitupdesc;   /* rowtype descriptor of xs_hitup */
 
    /* xs_ctup/xs_cbuf/xs_recheck are valid after a successful index_getnext */
    HeapTupleData xs_ctup;      /* current heap tuple, if any */
index e42079b09f848cdd4bffcf1a041afd663d82c40f..4072c050dea7e7f84c98e1347e57b883a3d2a767 100644 (file)
@@ -159,7 +159,7 @@ typedef struct SpGistScanOpaqueData
    int         iPtr;           /* index for scanning through same */
    ItemPointerData heapPtrs[MaxIndexTuplesPerPage];    /* TIDs from cur page */
    bool        recheck[MaxIndexTuplesPerPage]; /* their recheck flags */
-   IndexTuple  indexTups[MaxIndexTuplesPerPage];       /* reconstructed tuples */
+   HeapTuple   reconTups[MaxIndexTuplesPerPage];       /* reconstructed tuples */
 
    /*
     * Note: using MaxIndexTuplesPerPage above is a bit hokey since