database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-06-16 06:01:02 +03:00
Code Activity

Minor cleanups in the BRIN code

Browse Source 
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
Download Patch File Download Diff File Expand all files Collapse all files

15
src/backend/access/brin/brin_bloom.c
View File

@ -574,7 +574,7 @@ brin_bloom_consistent(PG_FUNCTION_ARGS)
Oid colloid = PG_GET_COLLATION(); Oid colloid = PG_GET_COLLATION();
AttrNumber attno; AttrNumber attno;
Datum value; Datum value;
Datum matches; bool matches;
FmgrInfo *finfo; FmgrInfo *finfo;
uint32 hashValue; uint32 hashValue;
BloomFilter *filter; BloomFilter *filter;
@ -584,6 +584,10 @@ brin_bloom_consistent(PG_FUNCTION_ARGS)
Assert(filter); 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; matches = true;
for (keyno = 0; keyno < nkeys; keyno++) for (keyno = 0; keyno < nkeys; keyno++)
@ -601,9 +605,8 @@ brin_bloom_consistent(PG_FUNCTION_ARGS)
case BloomEqualStrategyNumber: case BloomEqualStrategyNumber:
/* /*
* In the equality case (WHERE col = someval), we want to * We want to return the current page range if the bloom filter
* return the current page range if the minimum value in the * seems to contain the value.
* range <= scan key, and the maximum value >= scan key.
*/ */
finfo = bloom_get_procinfo(bdesc, attno, PROCNUM_HASH); finfo = bloom_get_procinfo(bdesc, attno, PROCNUM_HASH);
@ -614,7 +617,7 @@ brin_bloom_consistent(PG_FUNCTION_ARGS)
default: default:
/* shouldn't happen */ /* shouldn't happen */
elog(ERROR, "invalid strategy number %d", key->sk_strategy); elog(ERROR, "invalid strategy number %d", key->sk_strategy);
matches = 0; matches = false;
break; break;
} }
@ -622,7 +625,7 @@ brin_bloom_consistent(PG_FUNCTION_ARGS)
break; break;
} }
PG_RETURN_DATUM(matches); PG_RETURN_BOOL(matches);
} }
/* /*

24
src/backend/access/brin/brin_minmax_multi.c
View File

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