From 8c0c35635a49a8233d6b7238b71e84cbc9007ea9 Mon Sep 17 00:00:00 2001 From: Emre Hasegeli Date: Sun, 14 Feb 2016 20:32:55 +0100 Subject: [PATCH] spgist fixed nnodes --- doc/src/sgml/spgist.sgml | 14 +++++++++---- src/backend/access/spgist/spgdoinsert.c | 36 ++++++++++++++++++++++++++------- src/backend/access/spgist/spgtextproc.c | 11 ++++++++-- src/include/access/spgist.h | 4 +++- 4 files changed, 51 insertions(+), 14 deletions(-) diff --git a/doc/src/sgml/spgist.sgml b/doc/src/sgml/spgist.sgml index 56827e5..b6441f8 100644 --- a/doc/src/sgml/spgist.sgml +++ b/doc/src/sgml/spgist.sgml @@ -321,23 +321,25 @@ typedef struct spgChooseOut struct /* results for spgAddNode */ { Datum nodeLabel; /* new node's label */ int nodeN; /* where to insert it (index from 0) */ } addNode; struct /* results for spgSplitTuple */ { /* Info to form new inner tuple with one node */ bool prefixHasPrefix; /* tuple should have a prefix? */ Datum prefixPrefixDatum; /* if so, its value */ - Datum nodeLabel; /* node's label */ - + int prefixNNodes; /* number of nodes for new inner tuple */ + Datum *prefixNodeLabels; /* their labels (or NULL for no labels) */ + /* Info to form new lower-level inner tuple with all old nodes */ + int postfixNodeN; /* where to insert it (index from 0) */ bool postfixHasPrefix; /* tuple should have a prefix? */ Datum postfixPrefixDatum; /* if so, its value */ } splitTuple; } result; } spgChooseOut; datum is the original datum that was to be inserted into the index. leafDatum is initially the same as @@ -398,22 +400,26 @@ typedef struct spgChooseOut set resultType to spgSplitTuple. This action moves all the existing nodes into a new lower-level inner tuple, and replaces the existing inner tuple with a tuple having a single node that links to the new lower-level inner tuple. Set prefixHasPrefix to indicate whether the new upper tuple should have a prefix, and if so set prefixPrefixDatum to the prefix value. This new prefix value must be sufficiently less restrictive than the original to accept the new value to be indexed, and it should be no longer than the original prefix. - Set nodeLabel to the label to be used for the - node that will point to the new lower-level inner tuple. + Set prefixNNodes and + prefixNodeLabels for the new prefix. + prefixNNodes must be at least 1 to + put the new lower-level inner tuple. + Set postfixNodeN that will point to the new + lower-level inner tuple. Set postfixHasPrefix to indicate whether the new lower-level inner tuple should have a prefix, and if so set postfixPrefixDatum to the prefix value. The combination of these two prefixes and the additional label must have the same meaning as the original prefix, because there is no opportunity to alter the node labels that are moved to the new lower-level tuple, nor to change any child index entries. After the node has been split, the choose function will be called again with the replacement inner tuple. That call will usually result in an spgAddNode result, diff --git a/src/backend/access/spgist/spgdoinsert.c b/src/backend/access/spgist/spgdoinsert.c index f090ca5..8658f5d 100644 --- a/src/backend/access/spgist/spgdoinsert.c +++ b/src/backend/access/spgist/spgdoinsert.c @@ -1699,30 +1699,46 @@ spgSplitNodeAction(Relation index, SpGistState *state, BlockNumber postfixBlkno; OffsetNumber postfixOffset; int i; spgxlogSplitTuple xlrec; Buffer newBuffer = InvalidBuffer; /* Should not be applied to nulls */ Assert(!SpGistPageStoresNulls(current->page)); /* - * Construct new prefix tuple, containing a single node with the specified - * label. (We'll update the node's downlink to point to the new postfix - * tuple, below.) + * Construct new prefix tuple with given number of nodes. We are + * going to update one of those node's downlink to point to the new + * postfix tuple, below. */ - node = spgFormNodeTuple(state, out->result.splitTuple.nodeLabel, false); + if (out->result.splitTuple.prefixNNodes == 0) + elog(ERROR, + "prefix must have at least one node to point to the new postfix"); + nodes = (SpGistNodeTuple *) palloc(sizeof(SpGistNodeTuple) * + out->result.splitTuple.prefixNNodes); + + for (i = 0; i < out->result.splitTuple.prefixNNodes; i++) + { + Datum label = (Datum) 0; + bool labelisnull = + (out->result.splitTuple.prefixNodeLabels == NULL); + + if (!labelisnull) + label = out->result.splitTuple.prefixNodeLabels[i]; + nodes[i] = spgFormNodeTuple(state, label, labelisnull); + } prefixTuple = spgFormInnerTuple(state, out->result.splitTuple.prefixHasPrefix, out->result.splitTuple.prefixPrefixDatum, - 1, &node); + out->result.splitTuple.prefixNNodes, + nodes); /* it must fit in the space that innerTuple now occupies */ if (prefixTuple->size > innerTuple->size) elog(ERROR, "SPGiST inner-tuple split must not produce longer prefix"); /* * Construct new postfix tuple, containing all nodes of innerTuple with * same node datums, but with the prefix specified by the picksplit * function. */ @@ -1800,24 +1816,30 @@ spgSplitNodeAction(Relation index, SpGistState *state, xlrec.postfixBlkSame = false; } /* * And set downlink pointer in the prefix tuple to point to postfix tuple. * (We can't avoid this step by doing the above two steps in opposite * order, because there might not be enough space on the page to insert * the postfix tuple first.) We have to update the local copy of the * prefixTuple too, because that's what will be written to WAL. */ - spgUpdateNodeLink(prefixTuple, 0, postfixBlkno, postfixOffset); + if (out->result.splitTuple.postfixNodeN >= + out->result.splitTuple.prefixNNodes) + elog(ERROR, + "prefix doesn't have enough nodes to point to the new postfix"); + spgUpdateNodeLink(prefixTuple, out->result.splitTuple.postfixNodeN, + postfixBlkno, postfixOffset); prefixTuple = (SpGistInnerTuple) PageGetItem(current->page, PageGetItemId(current->page, current->offnum)); - spgUpdateNodeLink(prefixTuple, 0, postfixBlkno, postfixOffset); + spgUpdateNodeLink(prefixTuple, out->result.splitTuple.postfixNodeN, + postfixBlkno, postfixOffset); MarkBufferDirty(current->buffer); if (RelationNeedsWAL(index)) { XLogRecPtr recptr; XLogBeginInsert(); XLogRegisterData((char *) &xlrec, sizeof(xlrec)); XLogRegisterData((char *) prefixTuple, prefixTuple->size); diff --git a/src/backend/access/spgist/spgtextproc.c b/src/backend/access/spgist/spgtextproc.c index e0d8f30..87edabe 100644 --- a/src/backend/access/spgist/spgtextproc.c +++ b/src/backend/access/spgist/spgtextproc.c @@ -205,23 +205,27 @@ spg_text_choose(PG_FUNCTION_ARGS) if (commonLen == 0) { out->result.splitTuple.prefixHasPrefix = false; } else { out->result.splitTuple.prefixHasPrefix = true; out->result.splitTuple.prefixPrefixDatum = formTextDatum(prefixStr, commonLen); } - out->result.splitTuple.nodeLabel = + out->result.splitTuple.prefixNNodes = 1; + out->result.splitTuple.prefixNodeLabels = + (Datum *) palloc(sizeof(Datum)); + out->result.splitTuple.prefixNodeLabels[0] = Int16GetDatum(*(unsigned char *) (prefixStr + commonLen)); + out->result.splitTuple.postfixNodeN = 0; if (prefixSize - commonLen == 1) { out->result.splitTuple.postfixHasPrefix = false; } else { out->result.splitTuple.postfixHasPrefix = true; out->result.splitTuple.postfixPrefixDatum = formTextDatum(prefixStr + commonLen + 1, prefixSize - commonLen - 1); @@ -273,21 +277,24 @@ spg_text_choose(PG_FUNCTION_ARGS) * labels as the original tuple. * * Note: it might seem tempting to shorten the upper tuple's prefix, * if it has one, then use its last byte as label for the lower tuple. * But that doesn't win since we know the incoming value matches the * whole prefix: we'd just end up splitting the lower tuple again. */ out->resultType = spgSplitTuple; out->result.splitTuple.prefixHasPrefix = in->hasPrefix; out->result.splitTuple.prefixPrefixDatum = in->prefixDatum; - out->result.splitTuple.nodeLabel = Int16GetDatum(-2); + out->result.splitTuple.prefixNNodes = 1; + out->result.splitTuple.prefixNodeLabels = (Datum *) palloc(sizeof(Datum)); + out->result.splitTuple.prefixNodeLabels[0] = Int16GetDatum(-2); + out->result.splitTuple.postfixNodeN = 0; out->result.splitTuple.postfixHasPrefix = false; } else { /* Add a node for the not-previously-seen nodeChar value */ out->resultType = spgAddNode; out->result.addNode.nodeLabel = Int16GetDatum(nodeChar); out->result.addNode.nodeN = i; } diff --git a/src/include/access/spgist.h b/src/include/access/spgist.h index 1994f71..8abc2b3 100644 --- a/src/include/access/spgist.h +++ b/src/include/access/spgist.h @@ -86,23 +86,25 @@ typedef struct spgChooseOut struct /* results for spgAddNode */ { Datum nodeLabel; /* new node's label */ int nodeN; /* where to insert it (index from 0) */ } addNode; struct /* results for spgSplitTuple */ { /* Info to form new inner tuple with one node */ bool prefixHasPrefix; /* tuple should have a prefix? */ Datum prefixPrefixDatum; /* if so, its value */ - Datum nodeLabel; /* node's label */ + int prefixNNodes; /* number of nodes for new inner tuple */ + Datum *prefixNodeLabels; /* their labels (or NULL for no labels) */ /* Info to form new lower-level inner tuple with all old nodes */ + int postfixNodeN; /* where to insert it (index from 0) */ bool postfixHasPrefix; /* tuple should have a prefix? */ Datum postfixPrefixDatum; /* if so, its value */ } splitTuple; } result; } spgChooseOut; /* * Argument structs for spg_picksplit method */ typedef struct spgPickSplitIn -- 2.5.4 (Apple Git-61)