From be7b3bb9ce6c0ab3937c074cd5497599c8a42590 Mon Sep 17 00:00:00 2001 From: Tomas Vondra Date: Sun, 9 Jun 2019 22:04:50 +0200 Subject: [PATCH 06/10] Pass all keys to BRIN consistent function at once --- src/backend/access/brin/brin.c | 125 ++++++++++++----- src/backend/access/brin/brin_inclusion.c | 164 +++++++++++++++-------- src/backend/access/brin/brin_minmax.c | 116 +++++++++++----- src/backend/access/brin/brin_validate.c | 4 +- src/include/catalog/pg_proc.dat | 4 +- 5 files changed, 292 insertions(+), 121 deletions(-) diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.c index 63f2d7990c..31d09bc88e 100644 --- a/src/backend/access/brin/brin.c +++ b/src/backend/access/brin/brin.c @@ -383,6 +383,9 @@ bringetbitmap(IndexScanDesc scan, TIDBitmap *tbm) BrinMemTuple *dtup; BrinTuple *btup = NULL; Size btupsz = 0; + ScanKey **keys; + int *nkeys; + int keyno; opaque = (BrinOpaque *) scan->opaque; bdesc = opaque->bo_bdesc; @@ -404,6 +407,53 @@ bringetbitmap(IndexScanDesc scan, TIDBitmap *tbm) */ consistentFn = palloc0(sizeof(FmgrInfo) * bdesc->bd_tupdesc->natts); + /* + * Make room for per-attribute lists of scan keys that we'll pass to the + * consistent support procedure. + */ + keys = palloc0(sizeof(ScanKey *) * bdesc->bd_tupdesc->natts); + nkeys = palloc0(sizeof(int) * bdesc->bd_tupdesc->natts); + + /* + * Preprocess the scan keys - split them into per-attribute arrays. + */ + for (keyno = 0; keyno < scan->numberOfKeys; keyno++) + { + ScanKey key = &scan->keyData[keyno]; + AttrNumber keyattno = key->sk_attno; + + /* + * The collation of the scan key must match the collation + * used in the index column (but only if the search is not + * IS NULL/ IS NOT NULL). Otherwise we shouldn't be using + * this index ... + */ + Assert((key->sk_flags & SK_ISNULL) || + (key->sk_collation == + TupleDescAttr(bdesc->bd_tupdesc, + keyattno - 1)->attcollation)); + + /* First time we see this index attribute, so init as needed. */ + if (!keys[keyattno-1]) + { + FmgrInfo *tmp; + + keys[keyattno-1] = palloc0(sizeof(ScanKey) * scan->numberOfKeys); + + /* First time this column, so look up consistent function */ + Assert(consistentFn[keyattno - 1].fn_oid == InvalidOid); + + tmp = index_getprocinfo(idxRel, keyattno, + BRIN_PROCNUM_CONSISTENT); + fmgr_info_copy(&consistentFn[keyattno - 1], tmp, + CurrentMemoryContext); + } + + /* Add key to the per-attribute array. */ + keys[keyattno - 1][nkeys[keyattno - 1]] = key; + nkeys[keyattno - 1]++; + } + /* allocate an initial in-memory tuple, out of the per-range memcxt */ dtup = brin_new_memtuple(bdesc); @@ -464,7 +514,7 @@ bringetbitmap(IndexScanDesc scan, TIDBitmap *tbm) } else { - int keyno; + int attno; /* * Compare scan keys with summary values stored for the range. @@ -474,34 +524,19 @@ bringetbitmap(IndexScanDesc scan, TIDBitmap *tbm) * no keys. */ addrange = true; - for (keyno = 0; keyno < scan->numberOfKeys; keyno++) + for (attno = 1; attno <= bdesc->bd_tupdesc->natts; attno++) { - ScanKey key = &scan->keyData[keyno]; - AttrNumber keyattno = key->sk_attno; - BrinValues *bval = &dtup->bt_columns[keyattno - 1]; + BrinValues *bval; Datum add; - /* - * The collation of the scan key must match the collation - * used in the index column (but only if the search is not - * IS NULL/ IS NOT NULL). Otherwise we shouldn't be using - * this index ... - */ - Assert((key->sk_flags & SK_ISNULL) || - (key->sk_collation == - TupleDescAttr(bdesc->bd_tupdesc, - keyattno - 1)->attcollation)); + /* skip attributes without any san keys */ + if (!nkeys[attno - 1]) + continue; - /* First time this column? look up consistent function */ - if (consistentFn[keyattno - 1].fn_oid == InvalidOid) - { - FmgrInfo *tmp; + bval = &dtup->bt_columns[attno - 1]; - tmp = index_getprocinfo(idxRel, keyattno, - BRIN_PROCNUM_CONSISTENT); - fmgr_info_copy(&consistentFn[keyattno - 1], tmp, - CurrentMemoryContext); - } + Assert((nkeys[attno - 1] > 0) && + (nkeys[attno - 1] <= scan->numberOfKeys)); /* * Check whether the scan key is consistent with the page @@ -513,12 +548,42 @@ bringetbitmap(IndexScanDesc scan, TIDBitmap *tbm) * the range as a whole, so break out of the loop as soon * as a false return value is obtained. */ - add = FunctionCall3Coll(&consistentFn[keyattno - 1], - key->sk_collation, - PointerGetDatum(bdesc), - PointerGetDatum(bval), - PointerGetDatum(key)); - addrange = DatumGetBool(add); + if (consistentFn[attno - 1].fn_nargs >= 4) + { + Oid collation; + + /* + * 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 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 */ + int keyno; + + 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; + } + } if (!addrange) break; diff --git a/src/backend/access/brin/brin_inclusion.c b/src/backend/access/brin/brin_inclusion.c index 86788024ef..f9c2918031 100644 --- a/src/backend/access/brin/brin_inclusion.c +++ b/src/backend/access/brin/brin_inclusion.c @@ -85,6 +85,8 @@ 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); /* @@ -252,53 +254,103 @@ brin_inclusion_consistent(PG_FUNCTION_ARGS) { BrinDesc *bdesc = (BrinDesc *) PG_GETARG_POINTER(0); BrinValues *column = (BrinValues *) PG_GETARG_POINTER(1); - ScanKey key = (ScanKey) PG_GETARG_POINTER(2); - Oid colloid = PG_GET_COLLATION(), - subtype; - Datum unionval; - AttrNumber attno; - Datum query; - FmgrInfo *finfo; - Datum result; - - Assert(key->sk_attno == column->bv_attno); + ScanKey *keys = (ScanKey *) PG_GETARG_POINTER(2); + int nkeys = PG_GETARG_INT32(3); + Oid colloid = PG_GET_COLLATION(); + int keyno; + bool matches; + bool regular_keys = false; - /* Handle IS NULL/IS NOT NULL tests. */ - if (key->sk_flags & SK_ISNULL) + /* + * First check if there are any IS NULL scan keys, and if we're + * violating them. In that case we can terminate early, without + * inspecting the ranges. + */ + for (keyno = 0; keyno < nkeys; keyno++) { - if (key->sk_flags & SK_SEARCHNULL) + ScanKey key = keys[keyno]; + + Assert(key->sk_attno == column->bv_attno); + + /* handle IS NULL/IS NOT NULL tests */ + if (key->sk_flags & SK_ISNULL) { - if (column->bv_allnulls || column->bv_hasnulls) - PG_RETURN_BOOL(true); + if (key->sk_flags & SK_SEARCHNULL) + { + if (column->bv_allnulls || column->bv_hasnulls) + continue; /* this key is fine, continue */ + + PG_RETURN_BOOL(false); + } + + /* + * For IS NOT NULL, we can only skip ranges that are known to have + * only nulls. + */ + if (key->sk_flags & SK_SEARCHNOTNULL) + { + if (column->bv_allnulls) + PG_RETURN_BOOL(false); + + continue; + } + + /* + * Neither IS NULL nor IS NOT NULL was used; assume all indexable + * operators are strict and return false. + */ PG_RETURN_BOOL(false); } - - /* - * For IS NOT NULL, we can only skip ranges that are known to have - * only nulls. - */ - if (key->sk_flags & SK_SEARCHNOTNULL) - PG_RETURN_BOOL(!column->bv_allnulls); - - /* - * Neither IS NULL nor IS NOT NULL was used; assume all indexable - * operators are strict and return false. - */ - PG_RETURN_BOOL(false); + else + /* note we have regular (non-NULL) scan keys */ + regular_keys = true; } - /* If it is all nulls, it cannot possibly be consistent. */ - if (column->bv_allnulls) + /* + * If the page range is all nulls, it cannot possibly be consistent if + * there are some regular scan keys. + */ + if (column->bv_allnulls && regular_keys) PG_RETURN_BOOL(false); + /* If there are no regular keys, the page range is considered consistent. */ + if (!regular_keys) + PG_RETURN_BOOL(true); + /* It has to be checked, if it contains elements that are not mergeable. */ if (DatumGetBool(column->bv_values[INCLUSION_UNMERGEABLE])) PG_RETURN_BOOL(true); - attno = key->sk_attno; - subtype = key->sk_subtype; - query = key->sk_argument; - unionval = column->bv_values[INCLUSION_UNION]; + matches = true; + + for (keyno = 0; keyno < nkeys; keyno++) + { + ScanKey key = keys[keyno]; + + /* ignore IS NULL/IS NOT NULL tests handled above */ + if (key->sk_flags & SK_ISNULL) + continue; + + matches = inclusion_consistent_key(bdesc, column, key, colloid); + + if (!matches) + break; + } + + PG_RETURN_BOOL(matches); +} + +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; + switch (key->sk_strategy) { /* @@ -318,49 +370,49 @@ brin_inclusion_consistent(PG_FUNCTION_ARGS) finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype, RTOverRightStrategyNumber); result = FunctionCall2Coll(finfo, colloid, unionval, query); - PG_RETURN_BOOL(!DatumGetBool(result)); + return !DatumGetBool(result); case RTOverLeftStrategyNumber: finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype, RTRightStrategyNumber); result = FunctionCall2Coll(finfo, colloid, unionval, query); - PG_RETURN_BOOL(!DatumGetBool(result)); + return !DatumGetBool(result); case RTOverRightStrategyNumber: finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype, RTLeftStrategyNumber); result = FunctionCall2Coll(finfo, colloid, unionval, query); - PG_RETURN_BOOL(!DatumGetBool(result)); + return !DatumGetBool(result); case RTRightStrategyNumber: finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype, RTOverLeftStrategyNumber); result = FunctionCall2Coll(finfo, colloid, unionval, query); - PG_RETURN_BOOL(!DatumGetBool(result)); + return !DatumGetBool(result); case RTBelowStrategyNumber: finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype, RTOverAboveStrategyNumber); result = FunctionCall2Coll(finfo, colloid, unionval, query); - PG_RETURN_BOOL(!DatumGetBool(result)); + return !DatumGetBool(result); case RTOverBelowStrategyNumber: finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype, RTAboveStrategyNumber); result = FunctionCall2Coll(finfo, colloid, unionval, query); - PG_RETURN_BOOL(!DatumGetBool(result)); + return !DatumGetBool(result); case RTOverAboveStrategyNumber: finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype, RTBelowStrategyNumber); result = FunctionCall2Coll(finfo, colloid, unionval, query); - PG_RETURN_BOOL(!DatumGetBool(result)); + return !DatumGetBool(result); case RTAboveStrategyNumber: finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype, RTOverBelowStrategyNumber); result = FunctionCall2Coll(finfo, colloid, unionval, query); - PG_RETURN_BOOL(!DatumGetBool(result)); + return !DatumGetBool(result); /* * Overlap and contains strategies @@ -379,7 +431,7 @@ brin_inclusion_consistent(PG_FUNCTION_ARGS) finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype, key->sk_strategy); result = FunctionCall2Coll(finfo, colloid, unionval, query); - PG_RETURN_DATUM(result); + return DatumGetBool(result); /* * Contained by strategies @@ -400,9 +452,9 @@ brin_inclusion_consistent(PG_FUNCTION_ARGS) RTOverlapStrategyNumber); result = FunctionCall2Coll(finfo, colloid, unionval, query); if (DatumGetBool(result)) - PG_RETURN_BOOL(true); + return true; - PG_RETURN_DATUM(column->bv_values[INCLUSION_CONTAINS_EMPTY]); + return DatumGetBool(column->bv_values[INCLUSION_CONTAINS_EMPTY]); /* * Adjacent strategy @@ -419,12 +471,12 @@ brin_inclusion_consistent(PG_FUNCTION_ARGS) RTOverlapStrategyNumber); result = FunctionCall2Coll(finfo, colloid, unionval, query); if (DatumGetBool(result)) - PG_RETURN_BOOL(true); + return true; finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype, - RTAdjacentStrategyNumber); + RTAdjacentStrategyNumber); result = FunctionCall2Coll(finfo, colloid, unionval, query); - PG_RETURN_DATUM(result); + return DatumGetBool(result); /* * Basic comparison strategies @@ -454,9 +506,9 @@ brin_inclusion_consistent(PG_FUNCTION_ARGS) RTRightStrategyNumber); result = FunctionCall2Coll(finfo, colloid, unionval, query); if (!DatumGetBool(result)) - PG_RETURN_BOOL(true); + return true; - PG_RETURN_DATUM(column->bv_values[INCLUSION_CONTAINS_EMPTY]); + return DatumGetBool(column->bv_values[INCLUSION_CONTAINS_EMPTY]); case RTSameStrategyNumber: case RTEqualStrategyNumber: @@ -464,30 +516,30 @@ brin_inclusion_consistent(PG_FUNCTION_ARGS) RTContainsStrategyNumber); result = FunctionCall2Coll(finfo, colloid, unionval, query); if (DatumGetBool(result)) - PG_RETURN_BOOL(true); + return true; - PG_RETURN_DATUM(column->bv_values[INCLUSION_CONTAINS_EMPTY]); + return DatumGetBool(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)) - PG_RETURN_BOOL(true); + return true; - PG_RETURN_DATUM(column->bv_values[INCLUSION_CONTAINS_EMPTY]); + return DatumGetBool(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); - PG_RETURN_BOOL(!DatumGetBool(result)); + return !DatumGetBool(result); default: /* shouldn't happen */ elog(ERROR, "invalid strategy number %d", key->sk_strategy); - PG_RETURN_BOOL(false); + return false; } } diff --git a/src/backend/access/brin/brin_minmax.c b/src/backend/access/brin/brin_minmax.c index ad0d18ed39..2266fef2f9 100644 --- a/src/backend/access/brin/brin_minmax.c +++ b/src/backend/access/brin/brin_minmax.c @@ -31,6 +31,8 @@ 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 @@ -147,47 +149,99 @@ brin_minmax_consistent(PG_FUNCTION_ARGS) { BrinDesc *bdesc = (BrinDesc *) PG_GETARG_POINTER(0); BrinValues *column = (BrinValues *) PG_GETARG_POINTER(1); - ScanKey key = (ScanKey) PG_GETARG_POINTER(2); - Oid colloid = PG_GET_COLLATION(), - subtype; - AttrNumber attno; - Datum value; + ScanKey *keys = (ScanKey *) PG_GETARG_POINTER(2); + int nkeys = PG_GETARG_INT32(3); + Oid colloid = PG_GET_COLLATION(); Datum matches; - FmgrInfo *finfo; + int keyno; + bool regular_keys = false; - Assert(key->sk_attno == column->bv_attno); - - /* handle IS NULL/IS NOT NULL tests */ - if (key->sk_flags & SK_ISNULL) + /* + * First check if there are any IS NULL scan keys, and if we're + * violating them. In that case we can terminate early, without + * inspecting the ranges. + */ + for (keyno = 0; keyno < nkeys; keyno++) { - if (key->sk_flags & SK_SEARCHNULL) + ScanKey key = keys[keyno]; + + Assert(key->sk_attno == column->bv_attno); + + /* handle IS NULL/IS NOT NULL tests */ + if (key->sk_flags & SK_ISNULL) { - if (column->bv_allnulls || column->bv_hasnulls) - PG_RETURN_BOOL(true); + if (key->sk_flags & SK_SEARCHNULL) + { + if (column->bv_allnulls || column->bv_hasnulls) + continue; /* this key is fine, continue */ + + PG_RETURN_BOOL(false); + } + + /* + * For IS NOT NULL, we can only skip ranges that are known to have + * only nulls. + */ + if (key->sk_flags & SK_SEARCHNOTNULL) + { + if (column->bv_allnulls) + PG_RETURN_BOOL(false); + + continue; + } + + /* + * Neither IS NULL nor IS NOT NULL was used; assume all indexable + * operators are strict and return false. + */ PG_RETURN_BOOL(false); } + else + /* note we have regular (non-NULL) scan keys */ + regular_keys = true; + } - /* - * For IS NOT NULL, we can only skip ranges that are known to have - * only nulls. - */ - if (key->sk_flags & SK_SEARCHNOTNULL) - PG_RETURN_BOOL(!column->bv_allnulls); - - /* - * Neither IS NULL nor IS NOT NULL was used; assume all indexable - * operators are strict and return false. - */ + /* + * If the page range is all nulls, it cannot possibly be consistent if + * there are some regular scan keys. + */ + if (column->bv_allnulls && regular_keys) PG_RETURN_BOOL(false); + + /* If there are no regular keys, the page range is considered consistent. */ + if (!regular_keys) + PG_RETURN_BOOL(true); + + matches = true; + + for (keyno = 0; keyno < nkeys; keyno++) + { + ScanKey key = keys[keyno]; + + /* ignore IS NULL/IS NOT NULL tests handled above */ + if (key->sk_flags & SK_ISNULL) + continue; + + matches = minmax_consistent_key(bdesc, column, key, colloid); + + /* found non-matching key */ + if (!matches) + break; } - /* if the range is all empty, it cannot possibly be consistent */ - if (column->bv_allnulls) - PG_RETURN_BOOL(false); + PG_RETURN_DATUM(matches); +} + +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; + Datum matches; - attno = key->sk_attno; - subtype = key->sk_subtype; - value = key->sk_argument; switch (key->sk_strategy) { case BTLessStrategyNumber: @@ -230,7 +284,7 @@ brin_minmax_consistent(PG_FUNCTION_ARGS) break; } - PG_RETURN_DATUM(matches); + return DatumGetBool(matches); } /* diff --git a/src/backend/access/brin/brin_validate.c b/src/backend/access/brin/brin_validate.c index 4222a1781f..257096c615 100644 --- a/src/backend/access/brin/brin_validate.c +++ b/src/backend/access/brin/brin_validate.c @@ -98,8 +98,8 @@ brinvalidate(Oid opclassoid) break; case BRIN_PROCNUM_CONSISTENT: ok = check_amproc_signature(procform->amproc, BOOLOID, true, - 3, 4, INTERNALOID, INTERNALOID, - INTERNALOID, INTERNALOID); + 3, 5, INTERNALOID, INTERNALOID, + INTERNALOID, INT4OID, INTERNALOID); break; case BRIN_PROCNUM_UNION: ok = check_amproc_signature(procform->amproc, BOOLOID, true, diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat index bc3d08caec..3fb4073367 100644 --- a/src/include/catalog/pg_proc.dat +++ b/src/include/catalog/pg_proc.dat @@ -7891,7 +7891,7 @@ prosrc => 'brin_minmax_add_value' }, { oid => '3385', descr => 'BRIN minmax support', proname => 'brin_minmax_consistent', prorettype => 'bool', - proargtypes => 'internal internal internal internal', + proargtypes => 'internal internal internal int4 internal', prosrc => 'brin_minmax_consistent' }, { oid => '3386', descr => 'BRIN minmax support', proname => 'brin_minmax_union', prorettype => 'bool', @@ -7907,7 +7907,7 @@ prosrc => 'brin_inclusion_add_value' }, { oid => '4107', descr => 'BRIN inclusion support', proname => 'brin_inclusion_consistent', prorettype => 'bool', - proargtypes => 'internal internal internal internal', + proargtypes => 'internal internal internal int4 internal', prosrc => 'brin_inclusion_consistent' }, { oid => '4108', descr => 'BRIN inclusion support', proname => 'brin_inclusion_union', prorettype => 'bool', -- 2.20.1