Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtp (Exim 4.80) (envelope-from ) id 1Y3M5m-0007vY-9S for pgsql-hackers@arkaria.postgresql.org; Tue, 23 Dec 2014 09:49:26 +0000 Received: from localhost ([127.0.0.1] helo=postgresql.org) by malur.postgresql.org with smtp (Exim 4.80) (envelope-from ) id 1Y3M5l-0003x2-PK for pgsql-hackers@arkaria.postgresql.org; Tue, 23 Dec 2014 09:49:25 +0000 Received: from makus.postgresql.org ([2001:4800:1501:1::229]) by malur.postgresql.org with esmtps (TLS1.2:DHE_RSA_AES_256_CBC_SHA256:256) (Exim 4.80) (envelope-from ) id 1Y3M5j-0003s9-Cf for pgsql-hackers@postgresql.org; Tue, 23 Dec 2014 09:49:23 +0000 Received: from smtp-outbound-2.vmware.com ([208.91.2.13]) by makus.postgresql.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.80) (envelope-from ) id 1Y3M5c-00072N-Do for pgsql-hackers@postgresql.org; Tue, 23 Dec 2014 09:49:18 +0000 Received: from sc9-mailhost1.vmware.com (sc9-mailhost1.vmware.com [10.113.161.71]) by smtp-outbound-2.vmware.com (Postfix) with ESMTP id 1C8EB2817B; Tue, 23 Dec 2014 01:49:14 -0800 (PST) Received: from EX13-CAS-007.vmware.com (EX13-CAS-007.vmware.com [10.113.191.57]) by sc9-mailhost1.vmware.com (Postfix) with ESMTP id 162B5192FD; Tue, 23 Dec 2014 01:49:14 -0800 (PST) Received: from EX13-MBX-024.vmware.com (10.113.191.44) by EX13-MBX-012.vmware.com (10.113.191.32) with Microsoft SMTP Server (TLS) id 15.0.913.22; Tue, 23 Dec 2014 01:48:55 -0800 Received: from [192.168.1.90] (10.113.160.246) by EX13-MBX-024.vmware.com (10.113.191.44) with Microsoft SMTP Server (TLS) id 15.0.913.22; Tue, 23 Dec 2014 01:48:54 -0800 Message-ID: <54993A76.7050308@vmware.com> Date: Tue, 23 Dec 2014 11:48:38 +0200 From: Heikki Linnakangas Organization: VMware User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Icedove/31.3.0 MIME-Version: 1.0 To: Teodor Sigaev , Pgsql Hackers Subject: Re: compress method for spgist - 2 References: <54907069.1030506@sigaev.ru> In-Reply-To: <54907069.1030506@sigaev.ru> Content-Type: multipart/mixed; boundary="------------020109080207090809060301" X-Originating-IP: [10.113.160.246] X-ClientProxiedBy: EX13-CAS-013.vmware.com (10.113.191.65) To EX13-MBX-024.vmware.com (10.113.191.44) X-Pg-Spam-Score: -4.9 (----) List-Archive: List-Help: List-ID: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: X-Mailing-List: pgsql-hackers Precedence: bulk Sender: pgsql-hackers-owner@postgresql.org --------------020109080207090809060301 Content-Type: text/plain; charset="koi8-r"; format=flowed Content-Transfer-Encoding: 7bit On 12/16/2014 07:48 PM, Teodor Sigaev wrote: > /* > * This struct is what we actually keep in index->rd_amcache. It includes > * static configuration information as well as the lastUsedPages cache. > */ > typedef struct SpGistCache > { > spgConfigOut config; /* filled in by opclass config method */ > > SpGistTypeDesc attType; /* type of input data and leaf values */ > SpGistTypeDesc attPrefixType; /* type of inner-tuple prefix values */ > SpGistTypeDesc attLabelType; /* type of node label values */ > > SpGistLUPCache lastUsedPages; /* local storage of last-used info */ > } SpGistCache; Now that the input data type and leaf data type can be different, which one is "attType"? It's the leaf data type, as the patch stands. I renamed that to attLeafType, and went fixing all the references to it. In most places it's just a matter of search & replace, but what about the reconstructed datum? In freeScanStackEntry, we assume that att[Leaf]Type is the datatype for reconstructedValue, but I believe assume elsewhere that reconstructedValue is of the same data type as the input. At least if the opclass supports index-only scans. I think we'll need a separate SpGistTypeDesc for the input type. Or perhaps a separate SpGistTypeDesc for the reconstructed value and an optional decompress method to turn the reconstructedValue back into an actual reconstructed input datum. Or something like that. Attached is a patch with the kibitzing I did so far. - Heikki --------------020109080207090809060301 Content-Type: text/x-diff; name="spgist_compress_method-4-heikki.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="spgist_compress_method-4-heikki.patch" diff --git a/doc/src/sgml/spgist.sgml b/doc/src/sgml/spgist.sgml index 56827e5..de158c3 100644 --- a/doc/src/sgml/spgist.sgml +++ b/doc/src/sgml/spgist.sgml @@ -201,20 +201,21 @@ There are five user-defined methods that an index operator class for - SP-GiST must provide. All five follow the convention - of accepting two internal arguments, the first of which is a - pointer to a C struct containing input values for the support method, - while the second argument is a pointer to a C struct where output values - must be placed. Four of the methods just return void, since - all their results appear in the output struct; but + SP-GiST must provide and one optional. All five mandatory + methos follow the convention of accepting two internal arguments, + the first of which is a pointer to a C struct containing input values for + the support method, while the second argument is a pointer to a C struct + where output values must be placed. Four of the methods just return + void, since all their results appear in the output struct; but leaf_consistent additionally returns a boolean result. The methods must not modify any fields of their input structs. In all cases, the output struct is initialized to zeroes before calling the - user-defined method. + user-defined method. Optional method compress accepts + datum to be indexed and returns values which actually will be indexed. - The five user-defined methods are: + The five mandatory user-defined methods are: @@ -244,6 +245,7 @@ typedef struct spgConfigOut { Oid prefixType; /* Data type of inner-tuple prefixes */ Oid labelType; /* Data type of inner-tuple node labels */ + Oid leafType; /* Data type of leaf */ bool canReturnData; /* Opclass can reconstruct original data */ bool longValuesOK; /* Opclass can cope with values > 1 page */ } spgConfigOut; @@ -264,7 +266,15 @@ typedef struct spgConfigOut longValuesOK should be set true only when the attType is of variable length and the operator class is capable of segmenting long values by repeated suffixing - (see ). + (see ). leafType + usually has the same value as attType but if + it's different then optional method compress + should be provided. Method compress is responsible + for transformation from attType to + leafType. In this case all other function should + accept leafType values. Note: both consistent + functions will get scankeys unchanged, without + compress transformation. @@ -690,6 +700,24 @@ typedef struct spgLeafConsistentOut + + The optional user-defined method is: + + + + + Datum compress(Datum in) + + + Converts the data item into a format suitable for physical storage in + an index page. It accepts spgConfigIn.attType + value and return spgConfigOut.leafType + value. Output value should not be toasted. + + + + + All the SP-GiST support methods are normally called in a short-lived memory context; that is, CurrentMemoryContext will be reset diff --git a/src/backend/access/spgist/spgdoinsert.c b/src/backend/access/spgist/spgdoinsert.c index 1a17cc4..06c0680 100644 --- a/src/backend/access/spgist/spgdoinsert.c +++ b/src/backend/access/spgist/spgdoinsert.c @@ -1854,21 +1854,36 @@ spgdoinsert(Relation index, SpGistState *state, FmgrInfo *procinfo = NULL; /* - * Look up FmgrInfo of the user-defined choose function once, to save - * cycles in the loop below. + * Prepare the leaf datum to insert. + * + * If there is an optional "compress" method, call it to form the leaf + * datum from the input datum. Otherwise we will store the input datum as + * is. (We have to detoast it, though. We assume the "compress" method to + * return an untoasted value.) */ if (!isnull) - procinfo = index_getprocinfo(index, 1, SPGIST_CHOOSE_PROC); + { + if (OidIsValid(index_getprocid(index, 1, SPGIST_COMPRESS_PROC))) + { + procinfo = index_getprocinfo(index, 1, SPGIST_COMPRESS_PROC); + leafDatum = FunctionCall1Coll(procinfo, + index->rd_indcollation[0], + datum); + } + else if (state->attLeafType.attlen == -1) + leafDatum = PointerGetDatum(PG_DETOAST_DATUM(datum)); + else + leafDatum = datum; + } + else + leafDatum = (Datum) 0; /* - * Since we don't use index_form_tuple in this AM, we have to make sure - * value to be inserted is not toasted; FormIndexDatum doesn't guarantee - * that. + * Look up FmgrInfo of the user-defined choose function once, to save + * cycles in the loop below. */ - if (!isnull && state->attType.attlen == -1) - datum = PointerGetDatum(PG_DETOAST_DATUM(datum)); - - leafDatum = datum; + if (!isnull) + procinfo = index_getprocinfo(index, 1, SPGIST_CHOOSE_PROC); /* * Compute space needed for a leaf tuple containing the given datum. @@ -1878,7 +1893,7 @@ spgdoinsert(Relation index, SpGistState *state, */ if (!isnull) leafSize = SGLTHDRSZ + sizeof(ItemIdData) + - SpGistGetTypeSize(&state->attType, leafDatum); + SpGistGetTypeSize(&state->attLeafType, leafDatum); else leafSize = SGDTSIZE + sizeof(ItemIdData); @@ -2093,7 +2108,7 @@ spgdoinsert(Relation index, SpGistState *state, { leafDatum = out.result.matchNode.restDatum; leafSize = SGLTHDRSZ + sizeof(ItemIdData) + - SpGistGetTypeSize(&state->attType, leafDatum); + SpGistGetTypeSize(&state->attLeafType, leafDatum); } /* diff --git a/src/backend/access/spgist/spgscan.c b/src/backend/access/spgist/spgscan.c index 35cc41b..a4c5592 100644 --- a/src/backend/access/spgist/spgscan.c +++ b/src/backend/access/spgist/spgscan.c @@ -39,7 +39,8 @@ typedef struct ScanStackEntry static void freeScanStackEntry(SpGistScanOpaque so, ScanStackEntry *stackEntry) { - if (!so->state.attType.attbyval && + /* FIXME: Is attLeafType correct for reconstructedValue? */ + if (!so->state.attLeafType.attbyval && DatumGetPointer(stackEntry->reconstructedValue) != NULL) pfree(DatumGetPointer(stackEntry->reconstructedValue)); pfree(stackEntry); @@ -539,11 +540,14 @@ redirect: else newEntry->level = stackEntry->level; /* Must copy value out of temp context */ + /* + * FIXME: this assumes that the leaf data type is the same + * as the reconstructedValues datatype */ if (out.reconstructedValues) newEntry->reconstructedValue = datumCopy(out.reconstructedValues[i], - so->state.attType.attbyval, - so->state.attType.attlen); + so->state.attLeafType.attbyval, + so->state.attLeafType.attlen); else newEntry->reconstructedValue = (Datum) 0; diff --git a/src/backend/access/spgist/spgutils.c b/src/backend/access/spgist/spgutils.c index 1a224ef..dcbb30c 100644 --- a/src/backend/access/spgist/spgutils.c +++ b/src/backend/access/spgist/spgutils.c @@ -74,7 +74,21 @@ spgGetCache(Relation index) PointerGetDatum(&cache->config)); /* Get the information we need about each relevant datatype */ - fillTypeDesc(&cache->attType, atttype); + if (OidIsValid(cache->config.leafType) && + cache->config.leafType != atttype) + { + if (!OidIsValid(index_getprocid(index, 1, SPGIST_COMPRESS_PROC))) + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("compress method must not defined when leaf type is different from input type"))); + + fillTypeDesc(&cache->attLeafType, cache->config.leafType); + } + else + { + fillTypeDesc(&cache->attLeafType, atttype); + } + fillTypeDesc(&cache->attPrefixType, cache->config.prefixType); fillTypeDesc(&cache->attLabelType, cache->config.labelType); @@ -113,7 +127,7 @@ initSpGistState(SpGistState *state, Relation index) cache = spgGetCache(index); state->config = cache->config; - state->attType = cache->attType; + state->attLeafType = cache->attLeafType; state->attPrefixType = cache->attPrefixType; state->attLabelType = cache->attLabelType; @@ -556,7 +570,7 @@ spgFormLeafTuple(SpGistState *state, ItemPointer heapPtr, /* compute space needed (note result is already maxaligned) */ size = SGLTHDRSZ; if (!isnull) - size += SpGistGetTypeSize(&state->attType, datum); + size += SpGistGetTypeSize(&state->attLeafType, datum); /* * Ensure that we can replace the tuple with a dead tuple later. This @@ -572,7 +586,7 @@ spgFormLeafTuple(SpGistState *state, ItemPointer heapPtr, tup->nextOffset = InvalidOffsetNumber; tup->heapPtr = *heapPtr; if (!isnull) - memcpyDatum(SGLTDATAPTR(tup), &state->attType, datum); + memcpyDatum(SGLTDATAPTR(tup), &state->attLeafType, datum); return tup; } diff --git a/src/include/access/spgist.h b/src/include/access/spgist.h index 3aa96bd..bbf6d89 100644 --- a/src/include/access/spgist.h +++ b/src/include/access/spgist.h @@ -30,7 +30,8 @@ #define SPGIST_PICKSPLIT_PROC 3 #define SPGIST_INNER_CONSISTENT_PROC 4 #define SPGIST_LEAF_CONSISTENT_PROC 5 -#define SPGISTNProc 5 +#define SPGIST_COMPRESS_PROC 6 +#define SPGISTNProc 6 /* * Argument structs for spg_config method @@ -44,6 +45,7 @@ typedef struct spgConfigOut { Oid prefixType; /* Data type of inner-tuple prefixes */ Oid labelType; /* Data type of inner-tuple node labels */ + Oid leafType; /* Data type of leaf (type of SPGIST_COMPRESS_PROC output) */ bool canReturnData; /* Opclass can reconstruct original data */ bool longValuesOK; /* Opclass can cope with values > 1 page */ } spgConfigOut; diff --git a/src/include/access/spgist_private.h b/src/include/access/spgist_private.h index 4b6fdee..8ae7e75 100644 --- a/src/include/access/spgist_private.h +++ b/src/include/access/spgist_private.h @@ -115,13 +115,13 @@ typedef struct SpGistTypeDesc typedef struct SpGistState { - spgConfigOut config; /* filled in by opclass config method */ + spgConfigOut config; /* filled in by opclass config method */ - SpGistTypeDesc attType; /* type of input data and leaf values */ - SpGistTypeDesc attPrefixType; /* type of inner-tuple prefix values */ + SpGistTypeDesc attLeafType; /* type of leaf values */ + SpGistTypeDesc attPrefixType; /* type of inner-tuple prefix values */ SpGistTypeDesc attLabelType; /* type of node label values */ - char *deadTupleStorage; /* workspace for spgFormDeadTuple */ + char *deadTupleStorage; /* workspace for spgFormDeadTuple */ TransactionId myXid; /* XID to use when creating a redirect tuple */ bool isBuild; /* true if doing index build */ @@ -174,13 +174,13 @@ typedef SpGistScanOpaqueData *SpGistScanOpaque; */ typedef struct SpGistCache { - spgConfigOut config; /* filled in by opclass config method */ + spgConfigOut config; /* filled in by opclass config method */ - SpGistTypeDesc attType; /* type of input data and leaf values */ - SpGistTypeDesc attPrefixType; /* type of inner-tuple prefix values */ + SpGistTypeDesc attLeafType; /* type of leaf values */ + SpGistTypeDesc attPrefixType; /* type of inner-tuple prefix values */ SpGistTypeDesc attLabelType; /* type of node label values */ - SpGistLUPCache lastUsedPages; /* local storage of last-used info */ + SpGistLUPCache lastUsedPages; /* local storage of last-used info */ } SpGistCache; @@ -298,7 +298,7 @@ typedef SpGistLeafTupleData *SpGistLeafTuple; #define SGLTHDRSZ MAXALIGN(sizeof(SpGistLeafTupleData)) #define SGLTDATAPTR(x) (((char *) (x)) + SGLTHDRSZ) -#define SGLTDATUM(x, s) ((s)->attType.attbyval ? \ +#define SGLTDATUM(x, s) ((s)->attLeafType.attbyval ? \ *(Datum *) SGLTDATAPTR(x) : \ PointerGetDatum(SGLTDATAPTR(x))) diff --git a/src/include/catalog/pg_am.h b/src/include/catalog/pg_am.h index 67b57cd..6427552 100644 --- a/src/include/catalog/pg_am.h +++ b/src/include/catalog/pg_am.h @@ -129,7 +129,7 @@ DESCR("GiST index access method"); DATA(insert OID = 2742 ( gin 0 6 f f f f t t f f t f f 0 gininsert ginbeginscan - gingetbitmap ginrescan ginendscan ginmarkpos ginrestrpos ginbuild ginbuildempty ginbulkdelete ginvacuumcleanup - gincostestimate ginoptions )); DESCR("GIN index access method"); #define GIN_AM_OID 2742 -DATA(insert OID = 4000 ( spgist 0 5 f f f f f t f t f f f 0 spginsert spgbeginscan spggettuple spggetbitmap spgrescan spgendscan spgmarkpos spgrestrpos spgbuild spgbuildempty spgbulkdelete spgvacuumcleanup spgcanreturn spgcostestimate spgoptions )); +DATA(insert OID = 4000 ( spgist 0 6 f f f f f t f t f f f 0 spginsert spgbeginscan spggettuple spggetbitmap spgrescan spgendscan spgmarkpos spgrestrpos spgbuild spgbuildempty spgbulkdelete spgvacuumcleanup spgcanreturn spgcostestimate spgoptions )); DESCR("SP-GiST index access method"); #define SPGIST_AM_OID 4000 DATA(insert OID = 3580 ( brin 5 14 f f f f t t f t t f f 0 brininsert brinbeginscan - bringetbitmap brinrescan brinendscan brinmarkpos brinrestrpos brinbuild brinbuildempty brinbulkdelete brinvacuumcleanup - brincostestimate brinoptions )); --------------020109080207090809060301 Content-Type: text/plain Content-Disposition: inline Content-Transfer-Encoding: 8bit MIME-Version: 1.0 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers --------------020109080207090809060301--