Fix building of large (bigger than shared_buffers) hash indexes.

When the index is predicted to need more than NBuffers buckets,
CREATE INDEX attempts to sort the index entries by hash key before
insertion, so as to reduce thrashing.  This code path got broken by
commit 9f03ca915196dfc8, which overlooked that _hash_form_tuple() is not
just an alias for index_form_tuple().  The index got built anyway, but
with garbage data, so that searches for pre-existing tuples always
failed.  Fix by refactoring to separate construction of the indexable
data from calling index_form_tuple().

Per bug #14210 from Daniel Newman.  Back-patch to 9.5 where the
bug was introduced.

Report: <20160623162507.17237.39471@wrigleys.postgresql.org>

diff --git a/src/backend/access/hash/hash.c b/src/backend/access/hash/hash.c
index 49a6c816aabdcf9dd4fd7c4e6f80425e097c9375..19695ee1022485ff636b0ffb8dd6df0a4438777b 100644 (file)
--- a/src/backend/access/hash/hash.c
+++ b/src/backend/access/hash/hash.c
@@ -176,19 +176,25 @@ hashbuildCallback(Relation index,
                                  void *state)
 {
        HashBuildState *buildstate = (HashBuildState *) state;
+       Datum           index_values[1];
+       bool            index_isnull[1];
        IndexTuple      itup;
 
-       /* Hash indexes don't index nulls, see notes in hashinsert */
-       if (isnull[0])
+       /* convert data to a hash key; on failure, do not insert anything */
+       if (!_hash_convert_tuple(index,
+                                                        values, isnull,
+                                                        index_values, index_isnull))
                return;
 
        /* Either spool the tuple for sorting, or just put it into the index */
        if (buildstate->spool)
-               _h_spool(buildstate->spool, &htup->t_self, values, isnull);
+               _h_spool(buildstate->spool, &htup->t_self,
+                                index_values, index_isnull);
        else
        {
                /* form an index tuple and point it at the heap tuple */
-               itup = _hash_form_tuple(index, values, isnull);
+               itup = index_form_tuple(RelationGetDescr(index),
+                                                               index_values, index_isnull);
                itup->t_tid = htup->t_self;
                _hash_doinsert(index, itup);
                pfree(itup);
@@ -208,22 +214,18 @@ hashinsert(Relation rel, Datum *values, bool *isnull,
                   ItemPointer ht_ctid, Relation heapRel,
                   IndexUniqueCheck checkUnique)
 {
+       Datum           index_values[1];
+       bool            index_isnull[1];
        IndexTuple      itup;
 
-       /*
-        * If the single index key is null, we don't insert it into the index.
-        * Hash tables support scans on '='. Relational algebra says that A = B
-        * returns null if either A or B is null.  This means that no
-        * qualification used in an index scan could ever return true on a null
-        * attribute.  It also means that indices can't be used by ISNULL or
-        * NOTNULL scans, but that's an artifact of the strategy map architecture
-        * chosen in 1986, not of the way nulls are handled here.
-        */
-       if (isnull[0])
+       /* convert data to a hash key; on failure, do not insert anything */
+       if (!_hash_convert_tuple(rel,
+                                                        values, isnull,
+                                                        index_values, index_isnull))
                return false;
 
-       /* generate an index tuple */
-       itup = _hash_form_tuple(rel, values, isnull);
+       /* form an index tuple and point it at the heap tuple */
+       itup = index_form_tuple(RelationGetDescr(rel), index_values, index_isnull);
        itup->t_tid = *ht_ctid;
 
        _hash_doinsert(rel, itup);

diff --git a/src/backend/access/hash/hashutil.c b/src/backend/access/hash/hashutil.c
index 456954b06311b920c650d11d8158ec5354753f04..822862db7a784dac983e545ed056d734e9fae66b 100644 (file)
--- a/src/backend/access/hash/hashutil.c
+++ b/src/backend/access/hash/hashutil.c
@@ -240,27 +240,37 @@ _hash_get_indextuple_hashkey(IndexTuple itup)
 }
 
 /*
- * _hash_form_tuple - form an index tuple containing hash code only
+ * _hash_convert_tuple - convert raw index data to hash key
+ *
+ * Inputs: values and isnull arrays for the user data column(s)
+ * Outputs: values and isnull arrays for the index tuple, suitable for
+ *             passing to index_form_tuple().
+ *
+ * Returns true if successful, false if not (because there are null values).
+ * On a false result, the given data need not be indexed.
+ *
+ * Note: callers know that the index-column arrays are always of length 1.
+ * In principle, there could be more than one input column, though we do not
+ * currently support that.
  */
-IndexTuple
-_hash_form_tuple(Relation index, Datum *values, bool *isnull)
+bool
+_hash_convert_tuple(Relation index,
+                                       Datum *user_values, bool *user_isnull,
+                                       Datum *index_values, bool *index_isnull)
 {
-       IndexTuple      itup;
        uint32          hashkey;
-       Datum           hashkeydatum;
-       TupleDesc       hashdesc;
 
-       if (isnull[0])
-               hashkeydatum = (Datum) 0;
-       else
-       {
-               hashkey = _hash_datum2hashkey(index, values[0]);
-               hashkeydatum = UInt32GetDatum(hashkey);
-       }
-       hashdesc = RelationGetDescr(index);
-       Assert(hashdesc->natts == 1);
-       itup = index_form_tuple(hashdesc, &hashkeydatum, isnull);
-       return itup;
+       /*
+        * We do not insert null values into hash indexes.  This is okay because
+        * the only supported search operator is '=', and we assume it is strict.
+        */
+       if (user_isnull[0])
+               return false;
+
+       hashkey = _hash_datum2hashkey(index, user_values[0]);
+       index_values[0] = UInt32GetDatum(hashkey);
+       index_isnull[0] = false;
+       return true;
 }
 
 /*

diff --git a/src/include/access/hash.h b/src/include/access/hash.h
index fa3f9b61caa1273039cea3ba438ee9fbf2d5d593..ce314180e6b2fc187cbda3f9f13125d523bb0ac4 100644 (file)
--- a/src/include/access/hash.h
+++ b/src/include/access/hash.h
@@ -359,8 +359,9 @@ extern Bucket _hash_hashkey2bucket(uint32 hashkey, uint32 maxbucket,
 extern uint32 _hash_log2(uint32 num);
 extern void _hash_checkpage(Relation rel, Buffer buf, int flags);
 extern uint32 _hash_get_indextuple_hashkey(IndexTuple itup);
-extern IndexTuple _hash_form_tuple(Relation index,
-                                Datum *values, bool *isnull);
+extern bool _hash_convert_tuple(Relation index,
+                                       Datum *user_values, bool *user_isnull,
+                                       Datum *index_values, bool *index_isnull);
 extern OffsetNumber _hash_binsearch(Page page, uint32 hash_value);
 extern OffsetNumber _hash_binsearch_last(Page page, uint32 hash_value);