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
This commit is contained in:
Tomas Vondra 2023-07-02 10:18:56 +02:00
parent 4f49b3f849
commit 28d03feac3
2 changed files with 21 additions and 18 deletions

View File

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

View File

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