Fix bugs in GIN "fast scan" with partial match.

There were a couple of bugs here. First, if the fuzzy limit was exceeded,
the loop in entryGetItem might drop out too soon if a whole block needs to
be skipped because it's < advancePast ("continue" in a while-loop checks the
loop condition too). Secondly, the loop checked when stepping to a new page
that there is at least one offset on the page < advancePast, but we cannot
rely on that on subsequent calls of entryGetItem, because advancePast might
change in between. That caused the skipping loop to read bogus items in the
TbmIterateResult's offset array.

First item and fix by Alexander Korotkov, second bug pointed out by Fabrízio
de Royes Mello, by a small variation of Alexander's test query.

diff --git a/src/backend/access/gin/ginget.c b/src/backend/access/gin/ginget.c
index dfc3dc48c80412a056a303a5c146e7dc56c86c42..fda19cf4e69d73c3902bb9baf34e9157bcfb0c4d 100644 (file)
--- a/src/backend/access/gin/ginget.c
+++ b/src/backend/access/gin/ginget.c
@@ -729,11 +729,18 @@ entryGetItem(GinState *ginstate, GinScanEntry entry,
        /* A bitmap result */
        BlockNumber advancePastBlk = GinItemPointerGetBlockNumber(&advancePast);
        OffsetNumber advancePastOff = GinItemPointerGetOffsetNumber(&advancePast);
+       bool        gotitem = false;
 
        do
        {
-           if (entry->matchResult == NULL ||
-               entry->offset >= entry->matchResult->ntuples)
+           /*
+            * If we've exhausted all items on this block, move to next block
+            * in the bitmap.
+            */
+           while (entry->matchResult == NULL ||
+                  (entry->matchResult->ntuples >= 0 &&
+                   entry->offset >= entry->matchResult->ntuples) ||
+                  entry->matchResult->blockno < advancePastBlk)
            {
                entry->matchResult = tbm_iterate(entry->matchIterator);
 
@@ -746,18 +753,6 @@ entryGetItem(GinState *ginstate, GinScanEntry entry,
                    break;
                }
 
-               /*
-                * If all the matches on this page are <= advancePast, skip
-                * to next page.
-                */
-               if (entry->matchResult->blockno < advancePastBlk ||
-                   (entry->matchResult->blockno == advancePastBlk &&
-                    entry->matchResult->offsets[entry->offset] <= advancePastOff))
-               {
-                   entry->offset = entry->matchResult->ntuples;
-                   continue;
-               }
-
                /*
                 * Reset counter to the beginning of entry->matchResult. Note:
                 * entry->offset is still greater than matchResult->ntuples if
@@ -766,30 +761,43 @@ entryGetItem(GinState *ginstate, GinScanEntry entry,
                 */
                entry->offset = 0;
            }
+           if (entry->isFinished)
+               break;
 
+           /*
+            * We're now on the first page after advancePast which has any
+            * items on it. If it's a lossy result, return that.
+            */
            if (entry->matchResult->ntuples < 0)
            {
-               /*
-                * lossy result, so we need to check the whole page
-                */
                ItemPointerSetLossyPage(&entry->curItem,
                                        entry->matchResult->blockno);
 
                /*
                 * We might as well fall out of the loop; we could not
                 * estimate number of results on this page to support correct
-                * reducing of result even if it's enabled
+                * reducing of result even if it's enabled.
                 */
+               gotitem = true;
                break;
            }
-
+           /*
+            * Not a lossy page. Skip over any offsets <= advancePast, and
+            * return that.
+            */
            if (entry->matchResult->blockno == advancePastBlk)
            {
                /*
-                * Skip to the right offset on this page. We already checked
-                * in above loop that there is at least one item > advancePast
-                * on the page.
+                * First, do a quick check against the last offset on the page.
+                * If that's > advancePast, so are all the other offsets.
                 */
+               if (entry->matchResult->offsets[entry->matchResult->ntuples - 1] <= advancePastOff)
+               {
+                   entry->offset = entry->matchResult->ntuples;
+                   continue;
+               }
+
+               /* Otherwise scan to find the first item > advancePast */
                while (entry->matchResult->offsets[entry->offset] <= advancePastOff)
                    entry->offset++;
            }
@@ -798,7 +806,8 @@ entryGetItem(GinState *ginstate, GinScanEntry entry,
                           entry->matchResult->blockno,
                           entry->matchResult->offsets[entry->offset]);
            entry->offset++;
-       } while (entry->reduceResult == TRUE && dropItem(entry));
+           gotitem = true;
+       } while (!gotitem || (entry->reduceResult == TRUE && dropItem(entry)));
    }
    else if (!BufferIsValid(entry->buffer))
    {