From: Tomas Vondra Date: Sat, 12 Sep 2020 15:07:04 +0200 Subject: [PATCH 2/8] Pass all scan keys to BRIN consistent function at once Passing all scan keys to the BRIN consistent function at once may allow elimination of additional ranges, which would be impossible when only passing individual scan keys. The code continues to support both the original (one scan key at a time) and new (all scan keys at once) approaches, depending on whether the consistent function accepts three or four arguments. Author: Tomas Vondra Reviewed-by: Alvaro Herrera Reviewed-by: Mark Dilger Reviewed-by: Alexander Korotkov Discussion: https://postgr.es/m/c1138ead-7668-f0e1-0638-c3be3237e812@2ndquadrant.com --- src/backend/access/brin/brin.c | 158 +++++++++++++++++++----- src/backend/access/brin/brin_validate.c | 4 +- 2 files changed, 126 insertions(+), 36 deletions(-) diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.c index 27ba596c6e..f9a0476024 100644 --- a/src/backend/access/brin/brin.c +++ b/src/backend/access/brin/brin.c @@ -390,6 +390,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; @@ -411,6 +414,66 @@ 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. We allocate space for all attributes, so + * that we don't have to bother determining which attributes are used. + * + * XXX The widest table can have ~1600 attributes, so this may allocate a + * couple kilobytes of memory). We could invent a more compact approach + * (with just space for used attributes) but that would make the matching + * more complicated, so it may not be a win. + */ + 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; + + /* + * This is a bit of an overkill - we don't know how many scan keys + * are there for this attribute, so we simply allocate the largest + * number possible. This may waste a bit of memory, but we only + * expect small number of scan keys in general, so this should be + * negligible, and it's cheaper than having to repalloc + * repeatedly. + */ + 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); @@ -471,7 +534,7 @@ bringetbitmap(IndexScanDesc scan, TIDBitmap *tbm) } else { - int keyno; + int attno; /* * Compare scan keys with summary values stored for the range. @@ -481,51 +544,78 @@ 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 scan keys */ + if (nkeys[attno - 1] == 0) + 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 * range values; if so, have the pages in the range added * to the output bitmap. * - * 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. + * 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, as + * before. */ - 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 + * + * 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; + + 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); + + /* mismatching key, no need to look further */ + if (!addrange) + break; + } + } + if (!addrange) break; } diff --git a/src/backend/access/brin/brin_validate.c b/src/backend/access/brin/brin_validate.c index 6d4253c05e..11835d85cd 100644 --- a/src/backend/access/brin/brin_validate.c +++ b/src/backend/access/brin/brin_validate.c @@ -97,8 +97,8 @@ brinvalidate(Oid opclassoid) break; case BRIN_PROCNUM_CONSISTENT: ok = check_amproc_signature(procform->amproc, BOOLOID, true, - 3, 3, INTERNALOID, INTERNALOID, - INTERNALOID); + 3, 4, INTERNALOID, INTERNALOID, + INTERNALOID, INT4OID); break; case BRIN_PROCNUM_UNION: ok = check_amproc_signature(procform->amproc, BOOLOID, true, -- 2.26.2 --------------82D677E9F94AC7574E460205 Content-Type: text/x-patch; charset=UTF-8; name="0003-Process-all-scan-keys-in-existing-BRIN-opcl-20210308.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename*0="0003-Process-all-scan-keys-in-existing-BRIN-opcl-20210308.pa"; filename*1="tch"