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);
 }
 
 /*