diff --git a/doc/src/sgml/brin.sgml b/doc/src/sgml/brin.sgml index 078f51bb55a..4f15081674b 100644 --- a/doc/src/sgml/brin.sgml +++ b/doc/src/sgml/brin.sgml @@ -476,6 +476,19 @@ typedef struct BrinOpcInfo + + bool consistent(BrinDesc *bdesc, BrinValues *column, + ScanKey key) + + + Returns whether the ScanKey is consistent with the given indexed + values for a range. + The attribute number to use is passed as part of the scan key. + This is an older backward-compatible variant of the consistent function. + + + + bool addValue(BrinDesc *bdesc, BrinValues *column, Datum newval, bool isnull) diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.c index 0bf25fd05bd..2d8759c6c1a 100644 --- a/src/backend/access/brin/brin.c +++ b/src/backend/access/brin/brin.c @@ -634,26 +634,58 @@ bringetbitmap(IndexScanDesc scan, TIDBitmap *tbm) break; } + /* + * Collation from the first key (has to be the same for + * all keys for the same attribue). + */ + collation = keys[attno - 1][0]->sk_collation; + /* * Check whether the scan key is consistent with the page * range values; if so, have the pages in the range added * to the output bitmap. * - * XXX We simply use the collation from the first key (it - * has to be the same for all keys for the same attribue). + * The opclass may or may not support processing of multiple + * scan keys. We can determine that based on the number of + * arguments - functions with extra parameter (number of scan + * keys) do support this, otherwise we have to simply pass the + * scan keys one by one. */ - collation = keys[attno - 1][0]->sk_collation; + if (consistentFn[attno - 1].fn_nargs >= 4) + { + /* Check all keys at once */ + add = FunctionCall4Coll(&consistentFn[attno - 1], + collation, + PointerGetDatum(bdesc), + PointerGetDatum(bval), + PointerGetDatum(keys[attno - 1]), + Int32GetDatum(nkeys[attno - 1])); + addrange = DatumGetBool(add); + } + else + { + /* + * Check keys one by one + * + * When there are multiple scan keys, failure to meet the + * criteria for a single one of them is enough to discard + * the range as a whole, so break out of the loop as soon + * as a false return value is obtained. + */ + int keyno; - /* Check all keys at once */ - add = FunctionCall4Coll(&consistentFn[attno - 1], - collation, - PointerGetDatum(bdesc), - PointerGetDatum(bval), - PointerGetDatum(keys[attno - 1]), - Int32GetDatum(nkeys[attno - 1])); - addrange = DatumGetBool(add); - if (!addrange) - break; + for (keyno = 0; keyno < nkeys[attno - 1]; keyno++) + { + add = FunctionCall3Coll(&consistentFn[attno - 1], + keys[attno - 1][keyno]->sk_collation, + PointerGetDatum(bdesc), + PointerGetDatum(bval), + PointerGetDatum(keys[attno - 1][keyno])); + addrange = DatumGetBool(add); + if (!addrange) + break; + } + } } } } diff --git a/src/backend/access/brin/brin_inclusion.c b/src/backend/access/brin/brin_inclusion.c index d8cfc9d9098..0b384c0bd1e 100644 --- a/src/backend/access/brin/brin_inclusion.c +++ b/src/backend/access/brin/brin_inclusion.c @@ -85,8 +85,6 @@ static FmgrInfo *inclusion_get_procinfo(BrinDesc *bdesc, uint16 attno, uint16 procnum); static FmgrInfo *inclusion_get_strategy_procinfo(BrinDesc *bdesc, uint16 attno, Oid subtype, uint16 strategynum); -static bool inclusion_consistent_key(BrinDesc *bdesc, BrinValues *column, - ScanKey key, Oid colloid); /* @@ -243,9 +241,9 @@ brin_inclusion_add_value(PG_FUNCTION_ARGS) /* * BRIN inclusion consistent function * - * We inspect the IS NULL scan keys first, which allows us to make a decision - * without looking at the contents of the page range. Only when the page range - * matches the IS NULL keys, we check the regular scan keys. + * We're no longer dealing with NULL keys in the consistent function, that is + * now handled by the AM code. That means we should not get any all-NULL ranges + * either, because those can't be consistent with regular (not [IS] NULL) keys. * * All of the strategies are optional. */ @@ -254,63 +252,29 @@ brin_inclusion_consistent(PG_FUNCTION_ARGS) { BrinDesc *bdesc = (BrinDesc *) PG_GETARG_POINTER(0); BrinValues *column = (BrinValues *) PG_GETARG_POINTER(1); - ScanKey *keys = (ScanKey *) PG_GETARG_POINTER(2); - int nkeys = PG_GETARG_INT32(3); - Oid colloid = PG_GET_COLLATION(); - int keyno; + ScanKey key = (ScanKey) PG_GETARG_POINTER(2); + Oid colloid = PG_GET_COLLATION(), + subtype; + Datum unionval; + AttrNumber attno; + Datum query; + FmgrInfo *finfo; + Datum result; - /* make sure we got some scan keys */ - Assert((nkeys > 0) && (keys != NULL)); + /* This opclass uses the old signature with only three arguments. */ + Assert(PG_NARGS() == 3); - /* - * If is all nulls, it cannot possibly be consistent (at this point we - * know there are at least some regular scan keys). - */ - if (column->bv_allnulls) - PG_RETURN_BOOL(false); + /* Should not be dealing with all-NULL ranges. */ + Assert(!column->bv_allnulls); /* It has to be checked, if it contains elements that are not mergeable. */ if (DatumGetBool(column->bv_values[INCLUSION_UNMERGEABLE])) PG_RETURN_BOOL(true); - /* Check that the range is consistent with all regular scan keys. */ - for (keyno = 0; keyno < nkeys; keyno++) - { - ScanKey key = keys[keyno]; - - /* NULL keys are handled and filtered-out in bringetbitmap */ - Assert(!(key->sk_flags & SK_ISNULL)); - - /* - * When there are multiple scan keys, failure to meet the criteria for - * a single one of them is enough to discard the range as a whole, so - * break out of the loop as soon as a false return value is obtained. - */ - if (!inclusion_consistent_key(bdesc, column, key, colloid)) - PG_RETURN_BOOL(false); - } - - PG_RETURN_BOOL(true); -} - -/* - * inclusion_consistent_key - * Determine if the range is consistent with a single scan key. - */ -static bool -inclusion_consistent_key(BrinDesc *bdesc, BrinValues *column, ScanKey key, - Oid colloid) -{ - FmgrInfo *finfo; - AttrNumber attno = key->sk_attno; - Oid subtype = key->sk_subtype; - Datum query = key->sk_argument; - Datum unionval = column->bv_values[INCLUSION_UNION]; - Datum result; - - /* This should be called only for regular keys, not for IS [NOT] NULL. */ - Assert(!(key->sk_flags & SK_ISNULL)); - + attno = key->sk_attno; + subtype = key->sk_subtype; + query = key->sk_argument; + unionval = column->bv_values[INCLUSION_UNION]; switch (key->sk_strategy) { /* @@ -330,49 +294,49 @@ inclusion_consistent_key(BrinDesc *bdesc, BrinValues *column, ScanKey key, finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype, RTOverRightStrategyNumber); result = FunctionCall2Coll(finfo, colloid, unionval, query); - return !DatumGetBool(result); + PG_RETURN_BOOL(!DatumGetBool(result)); case RTOverLeftStrategyNumber: finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype, RTRightStrategyNumber); result = FunctionCall2Coll(finfo, colloid, unionval, query); - return !DatumGetBool(result); + PG_RETURN_BOOL(!DatumGetBool(result)); case RTOverRightStrategyNumber: finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype, RTLeftStrategyNumber); result = FunctionCall2Coll(finfo, colloid, unionval, query); - return !DatumGetBool(result); + PG_RETURN_BOOL(!DatumGetBool(result)); case RTRightStrategyNumber: finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype, RTOverLeftStrategyNumber); result = FunctionCall2Coll(finfo, colloid, unionval, query); - return !DatumGetBool(result); + PG_RETURN_BOOL(!DatumGetBool(result)); case RTBelowStrategyNumber: finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype, RTOverAboveStrategyNumber); result = FunctionCall2Coll(finfo, colloid, unionval, query); - return !DatumGetBool(result); + PG_RETURN_BOOL(!DatumGetBool(result)); case RTOverBelowStrategyNumber: finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype, RTAboveStrategyNumber); result = FunctionCall2Coll(finfo, colloid, unionval, query); - return !DatumGetBool(result); + PG_RETURN_BOOL(!DatumGetBool(result)); case RTOverAboveStrategyNumber: finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype, RTBelowStrategyNumber); result = FunctionCall2Coll(finfo, colloid, unionval, query); - return !DatumGetBool(result); + PG_RETURN_BOOL(!DatumGetBool(result)); case RTAboveStrategyNumber: finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype, RTOverBelowStrategyNumber); result = FunctionCall2Coll(finfo, colloid, unionval, query); - return !DatumGetBool(result); + PG_RETURN_BOOL(!DatumGetBool(result)); /* * Overlap and contains strategies @@ -390,7 +354,7 @@ inclusion_consistent_key(BrinDesc *bdesc, BrinValues *column, ScanKey key, finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype, key->sk_strategy); result = FunctionCall2Coll(finfo, colloid, unionval, query); - return DatumGetBool(result); + PG_RETURN_DATUM(result); /* * Contained by strategies @@ -410,9 +374,9 @@ inclusion_consistent_key(BrinDesc *bdesc, BrinValues *column, ScanKey key, RTOverlapStrategyNumber); result = FunctionCall2Coll(finfo, colloid, unionval, query); if (DatumGetBool(result)) - return true; + PG_RETURN_BOOL(true); - return DatumGetBool(column->bv_values[INCLUSION_CONTAINS_EMPTY]); + PG_RETURN_DATUM(column->bv_values[INCLUSION_CONTAINS_EMPTY]); /* * Adjacent strategy @@ -429,12 +393,12 @@ inclusion_consistent_key(BrinDesc *bdesc, BrinValues *column, ScanKey key, RTOverlapStrategyNumber); result = FunctionCall2Coll(finfo, colloid, unionval, query); if (DatumGetBool(result)) - return true; + PG_RETURN_BOOL(true); finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype, RTAdjacentStrategyNumber); result = FunctionCall2Coll(finfo, colloid, unionval, query); - return DatumGetBool(result); + PG_RETURN_DATUM(result); /* * Basic comparison strategies @@ -464,9 +428,9 @@ inclusion_consistent_key(BrinDesc *bdesc, BrinValues *column, ScanKey key, RTRightStrategyNumber); result = FunctionCall2Coll(finfo, colloid, unionval, query); if (!DatumGetBool(result)) - return true; + PG_RETURN_BOOL(true); - return DatumGetBool(column->bv_values[INCLUSION_CONTAINS_EMPTY]); + PG_RETURN_DATUM(column->bv_values[INCLUSION_CONTAINS_EMPTY]); case RTSameStrategyNumber: case RTEqualStrategyNumber: @@ -474,30 +438,30 @@ inclusion_consistent_key(BrinDesc *bdesc, BrinValues *column, ScanKey key, RTContainsStrategyNumber); result = FunctionCall2Coll(finfo, colloid, unionval, query); if (DatumGetBool(result)) - return true; + PG_RETURN_BOOL(true); - return DatumGetBool(column->bv_values[INCLUSION_CONTAINS_EMPTY]); + PG_RETURN_DATUM(column->bv_values[INCLUSION_CONTAINS_EMPTY]); case RTGreaterEqualStrategyNumber: finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype, RTLeftStrategyNumber); result = FunctionCall2Coll(finfo, colloid, unionval, query); if (!DatumGetBool(result)) - return true; + PG_RETURN_BOOL(true); - return DatumGetBool(column->bv_values[INCLUSION_CONTAINS_EMPTY]); + PG_RETURN_DATUM(column->bv_values[INCLUSION_CONTAINS_EMPTY]); case RTGreaterStrategyNumber: /* no need to check for empty elements */ finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype, RTLeftStrategyNumber); result = FunctionCall2Coll(finfo, colloid, unionval, query); - return !DatumGetBool(result); + PG_RETURN_BOOL(!DatumGetBool(result)); default: /* shouldn't happen */ elog(ERROR, "invalid strategy number %d", key->sk_strategy); - return false; + PG_RETURN_BOOL(false); } } diff --git a/src/backend/access/brin/brin_minmax.c b/src/backend/access/brin/brin_minmax.c index 032629950f1..798f06c7222 100644 --- a/src/backend/access/brin/brin_minmax.c +++ b/src/backend/access/brin/brin_minmax.c @@ -30,8 +30,6 @@ typedef struct MinmaxOpaque static FmgrInfo *minmax_get_strategy_procinfo(BrinDesc *bdesc, uint16 attno, Oid subtype, uint16 strategynum); -static bool minmax_consistent_key(BrinDesc *bdesc, BrinValues *column, - ScanKey key, Oid colloid); Datum @@ -133,64 +131,32 @@ brin_minmax_add_value(PG_FUNCTION_ARGS) * return whether the scan key is consistent with the index tuple's min/max * values. Return true if so, false otherwise. * - * We inspect the IS NULL scan keys first, which allows us to make a decision - * without looking at the contents of the page range. Only when the page range - * matches all those keys, we check the regular scan keys. + * We're no longer dealing with NULL keys in the consistent function, that is + * now handled by the AM code. That means we should not get any all-NULL ranges + * either, because those can't be consistent with regular (not [IS] NULL) keys. */ Datum brin_minmax_consistent(PG_FUNCTION_ARGS) { BrinDesc *bdesc = (BrinDesc *) PG_GETARG_POINTER(0); BrinValues *column = (BrinValues *) PG_GETARG_POINTER(1); - ScanKey *keys = (ScanKey *) PG_GETARG_POINTER(2); - int nkeys = PG_GETARG_INT32(3); - Oid colloid = PG_GET_COLLATION(); - int keyno; - - /* make sure we got some scan keys */ - Assert((nkeys > 0) && (keys != NULL)); - - /* - * If is all nulls, it cannot possibly be consistent (at this point we - * know there are at least some regular scan keys). - */ - if (column->bv_allnulls) - PG_RETURN_BOOL(false); - - /* Check that the range is consistent with all scan keys. */ - for (keyno = 0; keyno < nkeys; keyno++) - { - ScanKey key = keys[keyno]; - - /* NULL keys are handled and filtered-out in bringetbitmap */ - Assert(!(key->sk_flags & SK_ISNULL)); - - /* - * When there are multiple scan keys, failure to meet the criteria for - * a single one of them is enough to discard the range as a whole, so - * break out of the loop as soon as a false return value is obtained. - */ - if (!minmax_consistent_key(bdesc, column, key, colloid)) - PG_RETURN_DATUM(false); - } - - PG_RETURN_DATUM(true); -} - -/* - * minmax_consistent_key - * Determine if the range is consistent with a single scan key. - */ -static bool -minmax_consistent_key(BrinDesc *bdesc, BrinValues *column, ScanKey key, - Oid colloid) -{ - FmgrInfo *finfo; - AttrNumber attno = key->sk_attno; - Oid subtype = key->sk_subtype; - Datum value = key->sk_argument; + ScanKey key = (ScanKey) PG_GETARG_POINTER(2); + Oid colloid = PG_GET_COLLATION(), + subtype; + AttrNumber attno; + Datum value; Datum matches; + FmgrInfo *finfo; + /* This opclass uses the old signature with only three arguments. */ + Assert(PG_NARGS() == 3); + + /* Should not be dealing with all-NULL ranges. */ + Assert(!column->bv_allnulls); + + attno = key->sk_attno; + subtype = key->sk_subtype; + value = key->sk_argument; switch (key->sk_strategy) { case BTLessStrategyNumber: @@ -233,7 +199,7 @@ minmax_consistent_key(BrinDesc *bdesc, BrinValues *column, ScanKey key, break; } - return DatumGetBool(matches); + PG_RETURN_DATUM(matches); } /* diff --git a/src/backend/access/brin/brin_validate.c b/src/backend/access/brin/brin_validate.c index 2c4f9a3eff2..11835d85cd4 100644 --- a/src/backend/access/brin/brin_validate.c +++ b/src/backend/access/brin/brin_validate.c @@ -97,7 +97,7 @@ brinvalidate(Oid opclassoid) break; case BRIN_PROCNUM_CONSISTENT: ok = check_amproc_signature(procform->amproc, BOOLOID, true, - 4, 4, INTERNALOID, INTERNALOID, + 3, 4, INTERNALOID, INTERNALOID, INTERNALOID, INT4OID); break; case BRIN_PROCNUM_UNION: diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h index 872f0445fa7..7f8533c9367 100644 --- a/src/include/catalog/catversion.h +++ b/src/include/catalog/catversion.h @@ -53,6 +53,6 @@ */ /* yyyymmddN */ -#define CATALOG_VERSION_NO 202103261 +#define CATALOG_VERSION_NO 202103262 #endif diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat index 987ac9140b5..1662ee93ee3 100644 --- a/src/include/catalog/pg_proc.dat +++ b/src/include/catalog/pg_proc.dat @@ -8232,7 +8232,7 @@ prosrc => 'brin_minmax_add_value' }, { oid => '3385', descr => 'BRIN minmax support', proname => 'brin_minmax_consistent', prorettype => 'bool', - proargtypes => 'internal internal internal int4', + proargtypes => 'internal internal internal', prosrc => 'brin_minmax_consistent' }, { oid => '3386', descr => 'BRIN minmax support', proname => 'brin_minmax_union', prorettype => 'bool', @@ -8248,7 +8248,7 @@ prosrc => 'brin_inclusion_add_value' }, { oid => '4107', descr => 'BRIN inclusion support', proname => 'brin_inclusion_consistent', prorettype => 'bool', - proargtypes => 'internal internal internal int4', + proargtypes => 'internal internal internal', prosrc => 'brin_inclusion_consistent' }, { oid => '4108', descr => 'BRIN inclusion support', proname => 'brin_inclusion_union', prorettype => 'bool',