From: Heikki Linnakangas <hlinnakangas@vmware.com>
To: Teodor Sigaev <teodor@sigaev.ru>
To: Pgsql Hackers <pgsql-hackers@postgresql.org>
Subject: Re: compress method for spgist - 2
Date: Tue, 23 Dec 2014 11:48:38 +0200
Message-ID: <54993A76.7050308@vmware.com> (raw)
In-Reply-To: <54907069.1030506@sigaev.ru>
References: <54907069.1030506@sigaev.ru>
List-Unsubscribe: <mailto:majordomo@postgresql.org?body=unsub%20pgsql-hackers>
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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackersAttachments:spgist_compress_method-4-heikki.patch (text/x-diff, 13.4K) downloaddiff --git a/doc/src/sgml/spgist.sgml b/doc/src/sgml/spgist.sgmlindex 56827e5..de158c3 100644--- a/doc/src/sgml/spgist.sgml+++ b/doc/src/sgml/spgist.sgml@@ -201,20 +201,21 @@
<para>
There are five user-defined methods that an index operator class for
- <acronym>SP-GiST</acronym> must provide. All five follow the convention- of accepting two <type>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 <type>void</>, since- all their results appear in the output struct; but+ <acronym>SP-GiST</acronym> must provide and one optional. All five mandatory+ methos follow the convention of accepting two <type>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 + <type>void</>, since all their results appear in the output struct; but
<function>leaf_consistent</> additionally returns a <type>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 <function>compress</> accepts+ datum to be indexed and returns values which actually will be indexed.
</para>
<para>
- The five user-defined methods are:+ The five mandatory user-defined methods are:
</para>
<variablelist>
@@ -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
<structfield>longValuesOK</> should be set true only when the
<structfield>attType</> is of variable length and the operator
class is capable of segmenting long values by repeated suffixing
- (see <xref linkend="spgist-limits">).+ (see <xref linkend="spgist-limits">). <structfield>leafType</>+ usually has the same value as <structfield>attType</> but if+ it's different then optional method <function>compress</>+ should be provided. Method <function>compress</> is responsible+ for transformation from <structfield>attType</> to + <structfield>leafType</>. In this case all other function should+ accept <structfield>leafType</> values. Note: both consistent+ functions will get <structfield>scankeys</> unchanged, without+ <function>compress</> transformation.
</para>
</listitem>
</varlistentry>
@@ -690,6 +700,24 @@ typedef struct spgLeafConsistentOut
</varlistentry>
</variablelist>
+ <para>+ The optional user-defined method is:+ </para>++ <variablelist>+ <varlistentry>+ <term><function>Datum compress(Datum in)</></term>+ <listitem>+ <para>+ Converts the data item into a format suitable for physical storage in + an index page. It accepts <structname>spgConfigIn</>.<structfield>attType</>+ value and return <structname>spgConfigOut</>.<structfield>leafType</>+ value. Output value should not be toasted.+ </para>+ </listitem>+ </varlistentry>+ </variablelist>+
<para>
All the SP-GiST support methods are normally called in a short-lived
memory context; that is, <varname>CurrentMemoryContext</> will be reset
diff --git a/src/backend/access/spgist/spgdoinsert.c b/src/backend/access/spgist/spgdoinsert.cindex 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.cindex 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.cindex 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.hindex 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.hindex 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.hindex 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 ));
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: hlinnakangas@vmware.com, teodor@sigaev.ru
Subject: Re: compress method for spgist - 2
In-Reply-To: <54993A76.7050308@vmware.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox