From ed8b14643a8c313767f8285b40b44f96a9b8ece3 Mon Sep 17 00:00:00 2001 From: Tomas Vondra Date: Sun, 4 Feb 2018 14:27:42 +0100 Subject: [PATCH 2/4] Move IS [NOT] NULL checks to bringetbitmap These checks are exactly the same in all BRIN opclasses, so it makes sense to move them to bringetbimap() and evaluate them there. It also allows us to handle some ranges without invoking the consistent support procedure (e.g. when the key says IS NULL) but the range is marked as not containing any NULL values. Note: It may also be a good idea to first process NULL keys for all attributes on a given page range, and then only proceed with the remaining keys if it's still needed (when the page range is not eliminated by NULL keys on any attribute). --- src/backend/access/brin/brin.c | 116 ++++++++++++++++++++++++++++--- src/backend/access/brin/brin_inclusion.c | 62 +---------------- src/backend/access/brin/brin_minmax.c | 62 +---------------- 3 files changed, 109 insertions(+), 131 deletions(-) diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.c index b568786..7786f58 100644 --- a/src/backend/access/brin/brin.c +++ b/src/backend/access/brin/brin.c @@ -367,8 +367,10 @@ bringetbitmap(IndexScanDesc scan, TIDBitmap *tbm) BrinMemTuple *dtup; BrinTuple *btup = NULL; Size btupsz = 0; - ScanKey **keys; - int *nkeys; + ScanKey **keys, + **nullkeys; + int *nkeys, + *nnullkeys; int keyno; opaque = (BrinOpaque *) scan->opaque; @@ -393,10 +395,13 @@ bringetbitmap(IndexScanDesc scan, TIDBitmap *tbm) /* * Make room for per-attribute lists of scan keys that we'll pass to the - * consistent support procedure. + * consistent support procedure. We keep null and regular keys separate, + * so that we can easily pass regular keys to the consistent function. */ keys = palloc0(sizeof(ScanKey *) * bdesc->bd_tupdesc->natts); + nullkeys = palloc0(sizeof(ScanKey *) * bdesc->bd_tupdesc->natts); nkeys = palloc0(sizeof(int) * bdesc->bd_tupdesc->natts); + nnullkeys = palloc0(sizeof(int) * bdesc->bd_tupdesc->natts); /* * Preprocess the scan keys - split them into per-attribute arrays. @@ -418,14 +423,12 @@ bringetbitmap(IndexScanDesc scan, TIDBitmap *tbm) keyattno - 1)->attcollation)); /* First time we see this index attribute, so init as needed. */ - if (!keys[keyattno-1]) + if (consistentFn[keyattno - 1].fn_oid == InvalidOid) { 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); + Assert((keys[keyattno - 1] == NULL) && (nkeys[keyattno - 1] == 0)); + Assert((nullkeys[keyattno - 1] == NULL) && (nnullkeys[keyattno - 1] == 0)); tmp = index_getprocinfo(idxRel, keyattno, BRIN_PROCNUM_CONSISTENT); @@ -433,9 +436,23 @@ bringetbitmap(IndexScanDesc scan, TIDBitmap *tbm) CurrentMemoryContext); } - /* Add key to the per-attribute array. */ - keys[keyattno - 1][nkeys[keyattno - 1]] = key; - nkeys[keyattno - 1]++; + /* Add key to the proper per-attribute array. */ + if (key->sk_flags & SK_ISNULL) + { + if (!nullkeys[keyattno - 1]) + nullkeys[keyattno - 1] = palloc0(sizeof(ScanKey) * scan->numberOfKeys); + + nullkeys[keyattno - 1][nnullkeys[keyattno - 1]] = key; + nnullkeys[keyattno - 1]++; + } + else + { + if (!keys[keyattno - 1]) + keys[keyattno - 1] = palloc0(sizeof(ScanKey) * scan->numberOfKeys); + + keys[keyattno - 1][nkeys[keyattno - 1]] = key; + nkeys[keyattno - 1]++; + } } /* allocate an initial in-memory tuple, out of the per-range memcxt */ @@ -530,6 +547,83 @@ bringetbitmap(IndexScanDesc scan, TIDBitmap *tbm) collation = keys[attno - 1][0]->sk_collation; /* + * First check if there are any IS [NOT] NULL scan keys, and + * if we're violating them. In that case we can terminate + * early, without invoking the support function. + * + * As there may be more keys, we can only detemine mismatch + * within this loop. + */ + for (keyno = 0; (keyno < nnullkeys[attno - 1]); keyno++) + { + ScanKey key = nullkeys[attno - 1][keyno]; + + Assert(key->sk_attno == bval->bv_attno); + + /* interrupt the loop as soon as we find a mismatch */ + if (!addrange) + break; + + /* handle IS NULL/IS NOT NULL tests */ + if (key->sk_flags & SK_ISNULL) + { + /* IS NULL scan key, but range has no NULLs */ + if (key->sk_flags & SK_SEARCHNULL) + { + if (!bval->bv_allnulls && !bval->bv_hasnulls) + addrange = false; + + continue; + } + + /* + * For IS NOT NULL, we can only skip ranges that are + * known to have only nulls. + */ + if (key->sk_flags & SK_SEARCHNOTNULL) + { + if (bval->bv_allnulls) + addrange = false; + + continue; + } + + /* + * Neither IS NULL nor IS NOT NULL was used; assume all + * indexable operators are strict and thus return false + * with NULL value in the scan key. + */ + addrange = false; + } + } + + /* + * If any of the IS [NOT] NULL keys failed, the page range as + * a whole can't pass. So terminate the loop. + */ + if (!addrange) + break; + + /* + * So either there are no IS [NOT] NULL keys, or all passed. If + * there are no regular scan keys, we're done - the page range + * matches. If there are regular keys, but the page range is + * marked as 'all nulls' it can't possibly pass (we're assuming + * the operators are strict). + */ + + /* No regular scan keys - page range as a whole passes. */ + if (!nkeys[attno - 1]) + continue; + + /* If it is all nulls, it cannot possibly be consistent. */ + if (bval->bv_allnulls) + { + addrange = false; + break; + } + + /* * 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. diff --git a/src/backend/access/brin/brin_inclusion.c b/src/backend/access/brin/brin_inclusion.c index fb7f2b0..6d51c71 100644 --- a/src/backend/access/brin/brin_inclusion.c +++ b/src/backend/access/brin/brin_inclusion.c @@ -259,63 +259,6 @@ brin_inclusion_consistent(PG_FUNCTION_ARGS) Datum result; int keyno; bool matches; - bool regular_keys = false; - - /* - * 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++) - { - 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 (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; - } - - /* - * 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])) @@ -327,9 +270,8 @@ brin_inclusion_consistent(PG_FUNCTION_ARGS) { ScanKey key = keys[keyno]; - /* ignore IS NULL/IS NOT NULL tests handled above */ - if (key->sk_flags & SK_ISNULL) - continue; + /* NULL keys are handled and filtered-out in bringetbitmap */ + Assert(!(key->sk_flags & SK_ISNULL)); attno = key->sk_attno; subtype = key->sk_subtype; diff --git a/src/backend/access/brin/brin_minmax.c b/src/backend/access/brin/brin_minmax.c index e58fd16..4621bea 100644 --- a/src/backend/access/brin/brin_minmax.c +++ b/src/backend/access/brin/brin_minmax.c @@ -156,63 +156,6 @@ brin_minmax_consistent(PG_FUNCTION_ARGS) Datum matches; FmgrInfo *finfo; int keyno; - bool regular_keys = false; - - /* - * 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++) - { - 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 (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; - } - - /* - * 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; @@ -220,9 +163,8 @@ brin_minmax_consistent(PG_FUNCTION_ARGS) { ScanKey key = keys[keyno]; - /* ignore IS NULL/IS NOT NULL tests handled above */ - if (key->sk_flags & SK_ISNULL) - continue; + /* NULL keys are handled and filtered-out in bringetbitmap */ + Assert(!(key->sk_flags & SK_ISNULL)); attno = key->sk_attno; subtype = key->sk_subtype; -- 2.9.5