Initialize the entryRes array between each call to triConsistent.

The shimTriConstistentFn, which calls the opclass's consistent function with
all combinations of TRUE/FALSE for any MAYBE argument, modifies the entryRes
array passed by the caller. Change startScanKey to re-initialize it between
each call to accommodate that.

It's actually a bad habit by shimTriConsistentFn to modify its argument. But
the only caller that doesn't already re-initialize the entryRes array was
startScanKey, and it's easy for startScanKey to do so. Add a comment to
shimTriConsistentFn about that.

Note: this does not give a free pass to opclass-provided consistent
functions to modify the entryRes argument; shimTriConsistent assumes that
they don't, even though it does it itself.

While at it, refactor startScanKey to allocate the requiredEntries and
additionalEntries after it knows exactly how large they need to be. Saves a
little bit of memory, and looks nicer anyway.

Per complaint by Tom Lane, buildfarm and the pg_trgm regression test.

diff --git a/src/backend/access/gin/ginget.c b/src/backend/access/gin/ginget.c
index 1cb66d30a78343d9b86ee5f4e4e4fe3b034a8b97..e24ff180a73496628e761f420c801612fdcc0ed6 100644 (file)
--- a/src/backend/access/gin/ginget.c
+++ b/src/backend/access/gin/ginget.c
@@ -445,6 +445,7 @@ startScanKey(GinState *ginstate, GinScanOpaque so, GinScanKey key)
 {
        MemoryContext oldCtx = CurrentMemoryContext;
        int                     i;
+       int                     j;
        int                *entryIndexes;
 
        ItemPointerSetMin(&key->curItem);
@@ -472,10 +473,6 @@ startScanKey(GinState *ginstate, GinScanOpaque so, GinScanKey key)
         * entries can form a match, without any items from the required set. The
         * rest go to the additional set.
         */
-       key->requiredEntries = palloc(key->nentries * sizeof(GinScanEntry));
-       key->additionalEntries = palloc(key->nentries * sizeof(GinScanEntry));
-       key->nrequired = key->nadditional = 0;
-
        if (key->nentries > 1)
        {
                MemoryContextSwitchTo(so->tempCtx);
@@ -483,31 +480,44 @@ startScanKey(GinState *ginstate, GinScanOpaque so, GinScanKey key)
                entryIndexes = (int *) palloc(sizeof(int) * key->nentries);
                for (i = 0; i < key->nentries; i++)
                        entryIndexes[i] = i;
-
                qsort_arg(entryIndexes, key->nentries, sizeof(int),
                                  entryIndexByFrequencyCmp, key);
 
-               for (i = 0; i < key->nentries; i++)
-                       key->entryRes[i] = GIN_MAYBE;
-
-               for (i = 0; i < key->nentries; i++)
+               for (i = 0; i < key->nentries - 1; i++)
                {
-                       key->requiredEntries[key->nrequired++] = key->scanEntry[entryIndexes[i]];
-                       key->entryRes[entryIndexes[i]] = GIN_FALSE;
+                       /* Pass all entries <= i as FALSE, and the rest as MAYBE */
+                       for (j = 0; j <= i; j++)
+                               key->entryRes[entryIndexes[j]] = GIN_FALSE;
+                       for (j = i + 1; j < key->nentries; j++)
+                               key->entryRes[entryIndexes[j]] = GIN_MAYBE;
 
                        if (key->triConsistentFn(key) == GIN_FALSE)
                                break;
                }
-               for (i = i + 1; i < key->nentries; i++)
-                       key->additionalEntries[key->nadditional++] = key->scanEntry[entryIndexes[i]];
+               /* i is now the last required entry. */
 
-               /* clean up after consistentFn calls (also frees entryIndexes) */
                MemoryContextSwitchTo(oldCtx);
+
+               key->nrequired = i + 1;
+               key->nadditional = key->nentries - key->nrequired;
+               key->requiredEntries = palloc(key->nrequired * sizeof(GinScanEntry));
+               key->additionalEntries = palloc(key->nadditional * sizeof(GinScanEntry));
+
+               j = 0;
+               for (i = 0; i < key->nrequired; i++)
+                       key->requiredEntries[i] = key->scanEntry[entryIndexes[j++]];
+               for (i = 0; i < key->nadditional; i++)
+                       key->additionalEntries[i] = key->scanEntry[entryIndexes[j++]];
+
+               /* clean up after consistentFn calls (also frees entryIndexes) */
                MemoryContextReset(so->tempCtx);
        }
        else
        {
-               key->requiredEntries[key->nrequired++] = key->scanEntry[0];
+               key->nrequired = 1;
+               key->nadditional = 0;
+               key->requiredEntries = palloc(1 * sizeof(GinScanEntry));
+               key->requiredEntries[0] = key->scanEntry[0];
        }
 }
 

diff --git a/src/backend/access/gin/ginlogic.c b/src/backend/access/gin/ginlogic.c
index fb247446a2f925ee97e40ddbdbbb69357b24120e..dc8e6304a2f69bb5b136a079dccfff67a06640b9 100644 (file)
--- a/src/backend/access/gin/ginlogic.c
+++ b/src/backend/access/gin/ginlogic.c
@@ -90,6 +90,8 @@ normalBoolConsistentFn(GinScanKey key)
  * combination, that's the overall result. Otherwise, return MAYBE. Testing
  * every combination is O(n^2), so this is only feasible for a small number of
  * MAYBE inputs.
+ *
+ * NB: This function modifies the key->entryRes array!
  */
 static GinLogicValue
 shimTriConsistentFn(GinScanKey key)
@@ -98,7 +100,7 @@ shimTriConsistentFn(GinScanKey key)
        int                     maybeEntries[MAX_MAYBE_ENTRIES];
        int                     i;
        bool            boolResult;
-       bool            recheck = 0;
+       bool            recheck = false;
        GinLogicValue curResult;
 
        /*
@@ -124,7 +126,7 @@ shimTriConsistentFn(GinScanKey key)
        if (nmaybe == 0)
                return normalBoolConsistentFn(key);
 
-       /* Try the consistent function with the maybe-inputs set both ways */
+       /* First call consistent function with all the maybe-inputs set FALSE */
        for (i = 0; i < nmaybe; i++)
                key->entryRes[maybeEntries[i]] = GIN_FALSE;
        curResult = normalBoolConsistentFn(key);