From 2f2f011a3256724af658f87b030f1bfd017a235d Mon Sep 17 00:00:00 2001 From: Tomas Vondra Date: Tue, 14 Feb 2023 20:29:33 +0100 Subject: [PATCH 1/5] Introduce BRIN_PROCNUM_PREPROCESS procedure Allow BRIN opclasses to define an optional procedure to preprocess scan keys, and call it from brinrescan(). This allows the opclass to modify the keys in various ways - sort arrays, calculate hashes, ... Note: The procedure is optional, so existing opclasses don't need to add it. But if it uses the existing BRIN_PROCNUM_CONSISTENT function, it'll get broken. If we want to make this backwards-compatible, we might check if BRIN_PROCNUM_PREPROCESS exist from BRIN_PROCNUM_CONSISTENT, and adjust behavior based on that. The trouble is that amsearcharray=true is at the AM level, so it's not entirely optional. Discussion: https://postgr.es/m/f6aab55c-35eb-4118-a7ff-571c62e6cc39%40enterprisedb.com --- doc/src/sgml/xindex.sgml | 7 ++ src/backend/access/brin/brin.c | 154 +++++++++++++++++++++++++++-- src/include/access/brin_internal.h | 1 + 3 files changed, 153 insertions(+), 9 deletions(-) diff --git a/doc/src/sgml/xindex.sgml b/doc/src/sgml/xindex.sgml index c753d8005a..183a30cfb6 100644 --- a/doc/src/sgml/xindex.sgml +++ b/doc/src/sgml/xindex.sgml @@ -464,6 +464,13 @@ 5 + + + Preprocess scan keys in a way specific to this operator class + (optional) + + 6 + diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.c index 3c6a956eaa..0f773ad75d 100644 --- a/src/backend/access/brin/brin.c +++ b/src/backend/access/brin/brin.c @@ -60,12 +60,26 @@ typedef struct BrinBuildState /* * Struct used as "opaque" during index scans + * + * If the operator class preprocesses scan keys, the results are stored in + * bo_scanKeys. It's up to the operator class to decide which keys are worth + * preprocessing and which are simply copied. + * + * Either no keys are preprocessed (and ten bo_scanKeys is NULL), or all keys + * have a matching entry in bo_scanKeys (either preprocessed or a pointer to + * the original key). */ typedef struct BrinOpaque { BlockNumber bo_pagesPerRange; BrinRevmap *bo_rmAccess; BrinDesc *bo_bdesc; + + /* preprocessed scan keys */ + int bo_numScanKeys; /* number of (preprocessed) scan keys */ + ScanKey *bo_scanKeys; /* modified copy of scan->keyData */ + MemoryContext bo_scanKeysCxt; /* scan-lifespan context for key data */ + } BrinOpaque; #define BRIN_ALL_BLOCKRANGES InvalidBlockNumber @@ -335,6 +349,12 @@ brinbeginscan(Relation r, int nkeys, int norderbys) opaque->bo_rmAccess = brinRevmapInitialize(r, &opaque->bo_pagesPerRange, scan->xs_snapshot); opaque->bo_bdesc = brin_build_desc(r); + + /* no keys are preprocessed by default */ + opaque->bo_numScanKeys = 0; + opaque->bo_scanKeys = NULL; + opaque->bo_scanKeysCxt = NULL; + scan->opaque = opaque; return scan; @@ -377,6 +397,7 @@ bringetbitmap(IndexScanDesc scan, TIDBitmap *tbm) char *ptr; Size len; char *tmp PG_USED_FOR_ASSERTS_ONLY; + ScanKey *scankeys; opaque = (BrinOpaque *) scan->opaque; bdesc = opaque->bo_bdesc; @@ -454,10 +475,18 @@ bringetbitmap(IndexScanDesc scan, TIDBitmap *tbm) memset(nkeys, 0, sizeof(int) * bdesc->bd_tupdesc->natts); memset(nnullkeys, 0, sizeof(int) * bdesc->bd_tupdesc->natts); - /* Preprocess the scan keys - split them into per-attribute arrays. */ + /* get the preprocessed scan keys (if present) */ + scankeys = (opaque->bo_scanKeys) ? opaque->bo_scanKeys : &scan->keyData; + + /* + * Preprocess the scan keys - split them into per-attribute arrays. + * + * XXX A bit misleading, as this is a different kind of preprocessing of the + * scan keys. + */ for (int keyno = 0; keyno < scan->numberOfKeys; keyno++) { - ScanKey key = &scan->keyData[keyno]; + ScanKey key = scankeys[keyno]; AttrNumber keyattno = key->sk_attno; /* @@ -747,17 +776,124 @@ void brinrescan(IndexScanDesc scan, ScanKey scankey, int nscankeys, ScanKey orderbys, int norderbys) { - /* - * Other index AMs preprocess the scan keys at this point, or sometime - * early during the scan; this lets them optimize by removing redundant - * keys, or doing early returns when they are impossible to satisfy; see - * _bt_preprocess_keys for an example. Something like that could be added - * here someday, too. - */ + BrinOpaque *bo = (BrinOpaque *) scan->opaque; + Relation idxRel = scan->indexRelation; + MemoryContext oldcxt; + bool preprocess = false; if (scankey && scan->numberOfKeys > 0) memmove(scan->keyData, scankey, scan->numberOfKeys * sizeof(ScanKeyData)); + + /* + * Use the BRIN_PROCNUM_PREPROCESS procedure (if defined) to preprocess + * the scan keys. The procedure may do anything, as long as the result + * looks like a ScanKey. If there's no procedure, we keep the original + * scan keys. + * + * We only do this if the opclass (or at least one of them, for multi + * column indexes) defines BRIN_PROCNUM_PREPROCESS procedure. If none + * of them does, it's pointless to create the memory context etc. + * + * FIXME Probably needs fixes to handle NULLs correctly. + * + * XXX Maybe we should not ignore IS NULL scan keys? Could it make sense + * to preprocess those in some way? + */ + + /* + * Inspect if at least one scankey has BRIN_PROCNUM_PREPROCESS. + * + * Might seem wasteful, as this may require walking the scan keys twice + * (now and then also to do the preprocessing). But we stop on the first + * match and the number of keys should be small. Seems worth it if we + * can skip allocating the context. + */ + for (int i = 0; i < nscankeys; i++) + { + ScanKey key = &scan->keyData[i]; + Oid procid; + + /* skip IS NULL keys - there's nothing to preprocess */ + if (key->sk_flags & SK_ISNULL) + continue; + + /* fetch key preprocess support procedure if specified */ + procid = index_getprocid(idxRel, key->sk_attno, + BRIN_PROCNUM_PREPROCESS); + + /* don't look further if we found a preprocess procedure */ + if (OidIsValid(procid)) + { + preprocess = true; + break; + } + } + + /* No index attribute has preprocess procedure that we could use. */ + if (!preprocess) + return; + + /* + * Do the actual scan key preprocessing. If this is the first time + * through, we need to create the memory context. Otherwise we need + * to reset it, which throws away previously preprocessed keys. + */ + if (bo->bo_scanKeysCxt == NULL) + bo->bo_scanKeysCxt = AllocSetContextCreate(CurrentMemoryContext, + "BRIN scan keys context", + ALLOCSET_SMALL_SIZES); + else + MemoryContextReset(bo->bo_scanKeysCxt); + + oldcxt = MemoryContextSwitchTo(bo->bo_scanKeysCxt); + + bo->bo_scanKeys = palloc0(sizeof(ScanKey) * nscankeys); + + for (int i = 0; i < nscankeys; i++) + { + FmgrInfo *finfo; + ScanKey key = &scan->keyData[i]; + Oid procid; + Datum ret; + + /* + * If the scan argument is NULL, nothing to preprocess. + * + * XXX Maybe we should leave these checks up to the _preprocess + * procedures, in case there's something smart they wan to do? + * But SK_ISNULL is handled by bringetbitmap() so doing it here + * seems reasonable. + */ + if (key->sk_flags & SK_ISNULL) + { + bo->bo_scanKeys[i] = key; + continue; + } + + /* fetch key preprocess support procedure if specified */ + procid = index_getprocid(idxRel, key->sk_attno, + BRIN_PROCNUM_PREPROCESS); + + /* not specified, just point to the original key */ + if (!OidIsValid(procid)) + { + bo->bo_scanKeys[i] = key; + continue; + } + + /* preprocess the scan key and store the result */ + finfo = index_getprocinfo(idxRel, key->sk_attno, + BRIN_PROCNUM_PREPROCESS); + + ret = FunctionCall2(finfo, + PointerGetDatum(bo->bo_bdesc), + PointerGetDatum(key)); + + bo->bo_scanKeys[i] = (ScanKey) DatumGetPointer(ret); + } + + MemoryContextSwitchTo(oldcxt); } /* diff --git a/src/include/access/brin_internal.h b/src/include/access/brin_internal.h index 97ddc925b2..d6a51f2bc4 100644 --- a/src/include/access/brin_internal.h +++ b/src/include/access/brin_internal.h @@ -73,6 +73,7 @@ typedef struct BrinDesc #define BRIN_PROCNUM_UNION 4 #define BRIN_MANDATORY_NPROCS 4 #define BRIN_PROCNUM_OPTIONS 5 /* optional */ +#define BRIN_PROCNUM_PREPROCESS 6 /* optional */ /* procedure numbers up to 10 are reserved for BRIN future expansion */ #define BRIN_FIRST_OPTIONAL_PROCNUM 11 #define BRIN_LAST_OPTIONAL_PROCNUM 15 -- 2.41.0