Don't cast between GinNullCategory and bool

The original idea was that we could use an isNull-style bool array
directly as a GinNullCategory array.  However, the existing code already
acknowledges that that doesn't really work, because of the possibility
that bool as currently defined can have arbitrary bit patterns for true
values.  So it has to loop through the nullFlags array to set each bool
value to an acceptable value.  But if we are looping through the whole
array anyway, we might as well build a proper GinNullCategory array
instead and abandon the type casting.  That makes the code much safer in
case bool is ever changed to something else.

Reviewed-by: Michael Paquier <michael.paquier@gmail.com>

diff --git a/src/backend/access/gin/ginscan.c b/src/backend/access/gin/ginscan.c
index 7ceea7a741b0a74bcf3abd673ceceeb6c3539c06..77c0c577b50afd26bc85adfbca3d59f310e2242e 100644 (file)
--- a/src/backend/access/gin/ginscan.c
+++ b/src/backend/access/gin/ginscan.c
@@ -295,6 +295,7 @@ ginNewScanKey(IndexScanDesc scan)
                bool       *partial_matches = NULL;
                Pointer    *extra_data = NULL;
                bool       *nullFlags = NULL;
+               GinNullCategory *categories;
                int32           searchMode = GIN_SEARCH_MODE_DEFAULT;
 
                /*
@@ -346,15 +347,12 @@ ginNewScanKey(IndexScanDesc scan)
                }
 
                /*
-                * If the extractQueryFn didn't create a nullFlags array, create one,
-                * assuming that everything's non-null.  Otherwise, run through the
-                * array and make sure each value is exactly 0 or 1; this ensures
-                * binary compatibility with the GinNullCategory representation. While
-                * at it, detect whether any null keys are present.
+                * Create GinNullCategory representation.  If the extractQueryFn
+                * didn't create a nullFlags array, we assume everything is non-null.
+                * While at it, detect whether any null keys are present.
                 */
-               if (nullFlags == NULL)
-                       nullFlags = (bool *) palloc0(nQueryValues * sizeof(bool));
-               else
+               categories = (GinNullCategory *) palloc0(nQueryValues * sizeof(GinNullCategory));
+               if (nullFlags)
                {
                        int32           j;
 
@@ -362,17 +360,16 @@ ginNewScanKey(IndexScanDesc scan)
                        {
                                if (nullFlags[j])
                                {
-                                       nullFlags[j] = true;    /* not any other nonzero value */
+                                       categories[j] = GIN_CAT_NULL_KEY;
                                        hasNullQuery = true;
                                }
                        }
                }
-               /* now we can use the nullFlags as category codes */
 
                ginFillScanKey(so, skey->sk_attno,
                                           skey->sk_strategy, searchMode,
                                           skey->sk_argument, nQueryValues,
-                                          queryValues, (GinNullCategory *) nullFlags,
+                                          queryValues, categories,
                                           partial_matches, extra_data);
        }
 

diff --git a/src/backend/access/gin/ginutil.c b/src/backend/access/gin/ginutil.c
index d9c64834374781a7bd8f1c879031a72b48fb6e05..41d4b4fb6ffb3faff8d0655791c12ce084ffbb90 100644 (file)
--- a/src/backend/access/gin/ginutil.c
+++ b/src/backend/access/gin/ginutil.c
@@ -529,19 +529,10 @@ ginExtractEntries(GinState *ginstate, OffsetNumber attnum,
 
        /*
         * If the extractValueFn didn't create a nullFlags array, create one,
-        * assuming that everything's non-null.  Otherwise, run through the array
-        * and make sure each value is exactly 0 or 1; this ensures binary
-        * compatibility with the GinNullCategory representation.
+        * assuming that everything's non-null.
         */
        if (nullFlags == NULL)
                nullFlags = (bool *) palloc0(*nentries * sizeof(bool));
-       else
-       {
-               for (i = 0; i < *nentries; i++)
-                       nullFlags[i] = (nullFlags[i] ? true : false);
-       }
-       /* now we can use the nullFlags as category codes */
-       *categories = (GinNullCategory *) nullFlags;
 
        /*
         * If there's more than one key, sort and unique-ify.
@@ -600,6 +591,13 @@ ginExtractEntries(GinState *ginstate, OffsetNumber attnum,
                pfree(keydata);
        }
 
+       /*
+        * Create GinNullCategory representation from nullFlags.
+        */
+       *categories = (GinNullCategory *) palloc0(*nentries * sizeof(GinNullCategory));
+       for (i = 0; i < *nentries; i++)
+               (*categories)[i] = (nullFlags[i] ? GIN_CAT_NULL_KEY : GIN_CAT_NORM_KEY);
+
        return entries;
 }
 

diff --git a/src/include/access/ginblock.h b/src/include/access/ginblock.h
index 114370c7d719f482aa1e13f033e27b5aa56e07a6..c3af3f0380609eba690e9a4907c68ad43c82baf2 100644 (file)
--- a/src/include/access/ginblock.h
+++ b/src/include/access/ginblock.h
@@ -188,8 +188,11 @@ typedef struct
 
 /*
  * Category codes to distinguish placeholder nulls from ordinary NULL keys.
- * Note that the datatype size and the first two code values are chosen to be
- * compatible with the usual usage of bool isNull flags.
+ *
+ * The first two code values were chosen to be compatible with the usual usage
+ * of bool isNull flags.  However, casting between bool and GinNullCategory is
+ * risky because of the possibility of different bit patterns and type sizes,
+ * so it is no longer done.
  *
  * GIN_CAT_EMPTY_QUERY is never stored in the index; and notice that it is
  * chosen to sort before not after regular key values.