Minor cleanups in the BRIN code

BRIN bloom and minmax-multi opclasses were somewhat inconsistent when
dealing with bool variables, assigning to them Datum values etc. While
not a bug, it makes the code harder to understand, so fix that.

While at it, update an incorrect comment copied to bloom opclass from
minmax, talking about strategies not supported by bloom.

Reviewed-by: Heikki Linnakangas
Discussion: https://postgr.es/m/0e1f3350-c9cf-ab62-43a5-5dae314de89c%40enterprisedb.com

diff --git a/src/backend/access/brin/brin_bloom.c b/src/backend/access/brin/brin_bloom.c
index e4953a9d37b834fc0a67c8eb302179326c30d6a6..568faf1cd58ee872349dd1c6620a61cacc6197f8 100644 (file)
--- a/src/backend/access/brin/brin_bloom.c
+++ b/src/backend/access/brin/brin_bloom.c
@@ -574,7 +574,7 @@ brin_bloom_consistent(PG_FUNCTION_ARGS)
        Oid                     colloid = PG_GET_COLLATION();
        AttrNumber      attno;
        Datum           value;
-       Datum           matches;
+       bool            matches;
        FmgrInfo   *finfo;
        uint32          hashValue;
        BloomFilter *filter;
@@ -584,6 +584,10 @@ brin_bloom_consistent(PG_FUNCTION_ARGS)
 
        Assert(filter);
 
+       /*
+        * Assume all scan keys match. We'll be searching for a scan key eliminating
+        * the page range (we can stop on the first such key).
+        */
        matches = true;
 
        for (keyno = 0; keyno < nkeys; keyno++)
@@ -601,9 +605,8 @@ brin_bloom_consistent(PG_FUNCTION_ARGS)
                        case BloomEqualStrategyNumber:
 
                                /*
-                                * In the equality case (WHERE col = someval), we want to
-                                * return the current page range if the minimum value in the
-                                * range <= scan key, and the maximum value >= scan key.
+                                * We want to return the current page range if the bloom filter
+                                * seems to contain the value.
                                 */
                                finfo = bloom_get_procinfo(bdesc, attno, PROCNUM_HASH);
 
@@ -614,7 +617,7 @@ brin_bloom_consistent(PG_FUNCTION_ARGS)
                        default:
                                /* shouldn't happen */
                                elog(ERROR, "invalid strategy number %d", key->sk_strategy);
-                               matches = 0;
+                               matches = false;
                                break;
                }
 
@@ -622,7 +625,7 @@ brin_bloom_consistent(PG_FUNCTION_ARGS)
                        break;
        }
 
-       PG_RETURN_DATUM(matches);
+       PG_RETURN_BOOL(matches);
 }
 
 /*

diff --git a/src/backend/access/brin/brin_minmax_multi.c b/src/backend/access/brin/brin_minmax_multi.c
index 8e4e6c2fc8aa1e51d372a95a68da2bf296267d20..e9ce0f2a1be7c7b00d19bb690de3b13a16b1a52c 100644 (file)
--- a/src/backend/access/brin/brin_minmax_multi.c
+++ b/src/backend/access/brin/brin_minmax_multi.c
@@ -2602,7 +2602,7 @@ brin_minmax_multi_consistent(PG_FUNCTION_ARGS)
 
                for (keyno = 0; keyno < nkeys; keyno++)
                {
-                       Datum           matches;
+                       bool            matches;
                        ScanKey         key = keys[keyno];
 
                        /* NULL keys are handled and filtered-out in bringetbitmap */
@@ -2618,7 +2618,7 @@ brin_minmax_multi_consistent(PG_FUNCTION_ARGS)
                                        finfo = minmax_multi_get_strategy_procinfo(bdesc, attno, subtype,
                                                                                                                           key->sk_strategy);
                                        /* first value from the array */
-                                       matches = FunctionCall2Coll(finfo, colloid, minval, value);
+                                       matches = DatumGetBool(FunctionCall2Coll(finfo, colloid, minval, value));
                                        break;
 
                                case BTEqualStrategyNumber:
@@ -2664,18 +2664,18 @@ brin_minmax_multi_consistent(PG_FUNCTION_ARGS)
                                        finfo = minmax_multi_get_strategy_procinfo(bdesc, attno, subtype,
                                                                                                                           key->sk_strategy);
                                        /* last value from the array */
-                                       matches = FunctionCall2Coll(finfo, colloid, maxval, value);
+                                       matches = DatumGetBool(FunctionCall2Coll(finfo, colloid, maxval, value));
                                        break;
 
                                default:
                                        /* shouldn't happen */
                                        elog(ERROR, "invalid strategy number %d", key->sk_strategy);
-                                       matches = 0;
+                                       matches = false;
                                        break;
                        }
 
                        /* the range has to match all the scan keys */
-                       matching &= DatumGetBool(matches);
+                       matching &= matches;
 
                        /* once we find a non-matching key, we're done */
                        if (!matching)
@@ -2686,7 +2686,7 @@ brin_minmax_multi_consistent(PG_FUNCTION_ARGS)
                 * have we found a range matching all scan keys? if yes, we're done
                 */
                if (matching)
-                       PG_RETURN_DATUM(BoolGetDatum(true));
+                       PG_RETURN_BOOL(true);
        }
 
        /*
@@ -2703,7 +2703,7 @@ brin_minmax_multi_consistent(PG_FUNCTION_ARGS)
 
                for (keyno = 0; keyno < nkeys; keyno++)
                {
-                       Datum           matches;
+                       bool            matches;
                        ScanKey         key = keys[keyno];
 
                        /* we've already dealt with NULL keys at the beginning */
@@ -2723,18 +2723,18 @@ brin_minmax_multi_consistent(PG_FUNCTION_ARGS)
 
                                        finfo = minmax_multi_get_strategy_procinfo(bdesc, attno, subtype,
                                                                                                                           key->sk_strategy);
-                                       matches = FunctionCall2Coll(finfo, colloid, val, value);
+                                       matches = DatumGetBool(FunctionCall2Coll(finfo, colloid, val, value));
                                        break;
 
                                default:
                                        /* shouldn't happen */
                                        elog(ERROR, "invalid strategy number %d", key->sk_strategy);
-                                       matches = 0;
+                                       matches = false;
                                        break;
                        }
 
                        /* the range has to match all the scan keys */
-                       matching &= DatumGetBool(matches);
+                       matching &= matches;
 
                        /* once we find a non-matching key, we're done */
                        if (!matching)
@@ -2743,10 +2743,10 @@ brin_minmax_multi_consistent(PG_FUNCTION_ARGS)
 
                /* have we found a range matching all scan keys? if yes, we're done */
                if (matching)
-                       PG_RETURN_DATUM(BoolGetDatum(true));
+                       PG_RETURN_BOOL(true);
        }
 
-       PG_RETURN_DATUM(BoolGetDatum(false));
+       PG_RETURN_BOOL(false);
 }
 
 /*