Support multiple iterators in the Red-Black Tree implementation.

While we don't need multiple iterators at the moment, the interface is
nicer and less dangerous this way.

Aleksander Alekseev, with some changes by me.

diff --git a/src/backend/access/gin/ginbulk.c b/src/backend/access/gin/ginbulk.c
index d6422ea91ebdcd625fdb75ab4edd84367d9f1a3e..71c64e468c99249cdf03dec03eac43c9028ac9b6 100644 (file)
--- a/src/backend/access/gin/ginbulk.c
+++ b/src/backend/access/gin/ginbulk.c
@@ -255,7 +255,7 @@ qsortCompareItemPointers(const void *a, const void *b)
 void
 ginBeginBAScan(BuildAccumulator *accum)
 {
-       rb_begin_iterate(accum->tree, LeftRightWalk);
+       rb_begin_iterate(accum->tree, LeftRightWalk, &accum->tree_walk);
 }
 
 /*
@@ -271,7 +271,7 @@ ginGetBAEntry(BuildAccumulator *accum,
        GinEntryAccumulator *entry;
        ItemPointerData *list;
 
-       entry = (GinEntryAccumulator *) rb_iterate(accum->tree);
+       entry = (GinEntryAccumulator *) rb_iterate(&accum->tree_walk);
 
        if (entry == NULL)
                return NULL;                    /* no more entries */

diff --git a/src/backend/lib/rbtree.c b/src/backend/lib/rbtree.c
index 4fa8a1dd58a4da4e1f689e9beddac18adfd6e716..242e9803ccc54245046474671c174a3bb0e7937b 100644 (file)
--- a/src/backend/lib/rbtree.c
+++ b/src/backend/lib/rbtree.c
@@ -29,17 +29,6 @@
 #include "lib/rbtree.h"
 
 
-/*
- * Values of RBNode.iteratorState
- *
- * Note that iteratorState has an undefined value except in nodes that are
- * currently being visited by an active iteration.
- */
-#define InitialState   (0)
-#define FirstStepDone  (1)
-#define SecondStepDone (2)
-#define ThirdStepDone  (3)
-
 /*
  * Colors of nodes (values of RBNode.color)
  */
@@ -53,10 +42,6 @@ struct RBTree
 {
        RBNode     *root;                       /* root node, or RBNIL if tree is empty */
 
-       /* Iteration state */
-       RBNode     *cur;                        /* current iteration node */
-       RBNode     *(*iterate) (RBTree *rb);
-
        /* Remaining fields are constant after rb_create */
 
        Size            node_size;              /* actual size of tree nodes */
@@ -75,8 +60,19 @@ struct RBTree
  */
 #define RBNIL (&sentinel)
 
-static RBNode sentinel = {InitialState, RBBLACK, RBNIL, RBNIL, NULL};
+static RBNode sentinel = {RBBLACK, RBNIL, RBNIL, NULL};
 
+/*
+ * Values used in the RBTreeIterator.next_state field, with an
+ * InvertedWalk iterator.
+ */
+typedef enum InvertedWalkNextStep
+{
+       NextStepBegin,
+       NextStepUp,
+       NextStepLeft,
+       NextStepRight
+} InvertedWalkNextStep;
 
 /*
  * rb_create: create an empty RBTree
@@ -123,8 +119,6 @@ rb_create(Size node_size,
        Assert(node_size > sizeof(RBNode));
 
        tree->root = RBNIL;
-       tree->cur = RBNIL;
-       tree->iterate = NULL;
        tree->node_size = node_size;
        tree->comparator = comparator;
        tree->combiner = combiner;
@@ -437,7 +431,6 @@ rb_insert(RBTree *rb, const RBNode *data, bool *isNew)
 
        x = rb->allocfunc (rb->arg);
 
-       x->iteratorState = InitialState;
        x->color = RBRED;
 
        x->left = RBNIL;
@@ -653,171 +646,194 @@ rb_delete(RBTree *rb, RBNode *node)
  *                                               Traverse                                                                        *
  **********************************************************************/
 
-/*
- * The iterator routines were originally coded in tail-recursion style,
- * which is nice to look at, but is trouble if your compiler isn't smart
- * enough to optimize it.  Now we just use looping.
- */
-#define descend(next_node) \
-       do { \
-               (next_node)->iteratorState = InitialState; \
-               node = rb->cur = (next_node); \
-               goto restart; \
-       } while (0)
+static RBNode *
+rb_left_right_iterator(RBTreeIterator *iter)
+{
+       if (iter->last_visited == NULL)
+       {
+               iter->last_visited = iter->rb->root;
+               while (iter->last_visited->left != RBNIL)
+                       iter->last_visited = iter->last_visited->left;
 
-#define ascend(next_node) \
-       do { \
-               node = rb->cur = (next_node); \
-               goto restart; \
-       } while (0)
+               return iter->last_visited;
+       }
 
+       if (iter->last_visited->right != RBNIL)
+       {
+               iter->last_visited = iter->last_visited->right;
+               while (iter->last_visited->left != RBNIL)
+                       iter->last_visited = iter->last_visited->left;
 
-static RBNode *
-rb_left_right_iterator(RBTree *rb)
-{
-       RBNode     *node = rb->cur;
+               return iter->last_visited;
+       }
 
-restart:
-       switch (node->iteratorState)
+       for (;;)
        {
-               case InitialState:
-                       if (node->left != RBNIL)
-                       {
-                               node->iteratorState = FirstStepDone;
-                               descend(node->left);
-                       }
-                       /* FALL THROUGH */
-               case FirstStepDone:
-                       node->iteratorState = SecondStepDone;
-                       return node;
-               case SecondStepDone:
-                       if (node->right != RBNIL)
-                       {
-                               node->iteratorState = ThirdStepDone;
-                               descend(node->right);
-                       }
-                       /* FALL THROUGH */
-               case ThirdStepDone:
-                       if (node->parent)
-                               ascend(node->parent);
+               RBNode     *came_from = iter->last_visited;
+
+               iter->last_visited = iter->last_visited->parent;
+               if (iter->last_visited == NULL)
+               {
+                       iter->is_over = true;
                        break;
-               default:
-                       elog(ERROR, "unrecognized rbtree node state: %d",
-                                node->iteratorState);
+               }
+
+               if (iter->last_visited->left == came_from)
+                       break;                          /* came from left sub-tree, return current
+                                                                * node */
+
+               /* else - came from right sub-tree, continue to move up */
        }
 
-       return NULL;
+       return iter->last_visited;
 }
 
 static RBNode *
-rb_right_left_iterator(RBTree *rb)
+rb_right_left_iterator(RBTreeIterator *iter)
 {
-       RBNode     *node = rb->cur;
+       if (iter->last_visited == NULL)
+       {
+               iter->last_visited = iter->rb->root;
+               while (iter->last_visited->right != RBNIL)
+                       iter->last_visited = iter->last_visited->right;
+
+               return iter->last_visited;
+       }
 
-restart:
-       switch (node->iteratorState)
+       if (iter->last_visited->left != RBNIL)
        {
-               case InitialState:
-                       if (node->right != RBNIL)
-                       {
-                               node->iteratorState = FirstStepDone;
-                               descend(node->right);
-                       }
-                       /* FALL THROUGH */
-               case FirstStepDone:
-                       node->iteratorState = SecondStepDone;
-                       return node;
-               case SecondStepDone:
-                       if (node->left != RBNIL)
-                       {
-                               node->iteratorState = ThirdStepDone;
-                               descend(node->left);
-                       }
-                       /* FALL THROUGH */
-               case ThirdStepDone:
-                       if (node->parent)
-                               ascend(node->parent);
+               iter->last_visited = iter->last_visited->left;
+               while (iter->last_visited->right != RBNIL)
+                       iter->last_visited = iter->last_visited->right;
+
+               return iter->last_visited;
+       }
+
+       for (;;)
+       {
+               RBNode     *came_from = iter->last_visited;
+
+               iter->last_visited = iter->last_visited->parent;
+               if (iter->last_visited == NULL)
+               {
+                       iter->is_over = true;
                        break;
-               default:
-                       elog(ERROR, "unrecognized rbtree node state: %d",
-                                node->iteratorState);
+               }
+
+               if (iter->last_visited->right == came_from)
+                       break;                          /* came from right sub-tree, return current
+                                                                * node */
+
+               /* else - came from left sub-tree, continue to move up */
        }
 
-       return NULL;
+       return iter->last_visited;
 }
 
 static RBNode *
-rb_direct_iterator(RBTree *rb)
+rb_direct_iterator(RBTreeIterator *iter)
 {
-       RBNode     *node = rb->cur;
+       if (iter->last_visited == NULL)
+       {
+               iter->last_visited = iter->rb->root;
+               return iter->last_visited;
+       }
 
-restart:
-       switch (node->iteratorState)
+       if (iter->last_visited->left != RBNIL)
        {
-               case InitialState:
-                       node->iteratorState = FirstStepDone;
-                       return node;
-               case FirstStepDone:
-                       if (node->left != RBNIL)
+               iter->last_visited = iter->last_visited->left;
+               return iter->last_visited;
+       }
+
+       do
+       {
+               if (iter->last_visited->right != RBNIL)
+               {
+                       iter->last_visited = iter->last_visited->right;
+                       break;
+               }
+
+               /* go up and one step right */
+               for (;;)
+               {
+                       RBNode     *came_from = iter->last_visited;
+
+                       iter->last_visited = iter->last_visited->parent;
+                       if (iter->last_visited == NULL)
                        {
-                               node->iteratorState = SecondStepDone;
-                               descend(node->left);
+                               iter->is_over = true;
+                               break;
                        }
-                       /* FALL THROUGH */
-               case SecondStepDone:
-                       if (node->right != RBNIL)
+
+                       if ((iter->last_visited->right != came_from) && (iter->last_visited->right != RBNIL))
                        {
-                               node->iteratorState = ThirdStepDone;
-                               descend(node->right);
+                               iter->last_visited = iter->last_visited->right;
+                               return iter->last_visited;
                        }
-                       /* FALL THROUGH */
-               case ThirdStepDone:
-                       if (node->parent)
-                               ascend(node->parent);
-                       break;
-               default:
-                       elog(ERROR, "unrecognized rbtree node state: %d",
-                                node->iteratorState);
+               }
        }
+       while (iter->last_visited != NULL);
 
-       return NULL;
+       return iter->last_visited;
 }
 
 static RBNode *
-rb_inverted_iterator(RBTree *rb)
+rb_inverted_iterator(RBTreeIterator *iter)
 {
-       RBNode     *node = rb->cur;
+       RBNode     *came_from;
 
-restart:
-       switch (node->iteratorState)
+loop:
+       switch ((InvertedWalkNextStep) iter->next_step)
        {
-               case InitialState:
-                       if (node->left != RBNIL)
+                       /* First call, begin from root */
+               case NextStepBegin:
+                       iter->last_visited = iter->rb->root;
+                       iter->next_step = NextStepLeft;
+                       goto loop;
+
+               case NextStepLeft:
+                       while (iter->last_visited->left != RBNIL)
+                               iter->last_visited = iter->last_visited->left;
+
+                       iter->next_step = NextStepRight;
+                       goto loop;
+
+               case NextStepRight:
+                       if (iter->last_visited->right != RBNIL)
                        {
-                               node->iteratorState = FirstStepDone;
-                               descend(node->left);
+                               iter->last_visited = iter->last_visited->right;
+                               iter->next_step = NextStepLeft;
+                               goto loop;
                        }
-                       /* FALL THROUGH */
-               case FirstStepDone:
-                       if (node->right != RBNIL)
+                       else    /* not moved - return current, then go up */
+                               iter->next_step = NextStepUp;
+                       break;
+
+               case NextStepUp:
+                       for (;;)
                        {
-                               node->iteratorState = SecondStepDone;
-                               descend(node->right);
+                               came_from = iter->last_visited;
+                               iter->last_visited = iter->last_visited->parent;
+                               if (iter->last_visited == NULL)
+                               {
+                                       iter->is_over = true;
+                                       break;          /* end of iteration */
+                               }
+
+                               if (came_from == iter->last_visited->right)
+                               {
+                                       /* return current, then continue to go up */
+                                       break;
+                               }
+
+                               /* otherwise we came from the left */
+                               iter->next_step = NextStepRight;
+                               goto loop;
                        }
-                       /* FALL THROUGH */
-               case SecondStepDone:
-                       node->iteratorState = ThirdStepDone;
-                       return node;
-               case ThirdStepDone:
-                       if (node->parent)
-                               ascend(node->parent);
                        break;
-               default:
-                       elog(ERROR, "unrecognized rbtree node state: %d",
-                                node->iteratorState);
        }
 
-       return NULL;
+       return iter->last_visited;
 }
 
 /*
@@ -827,33 +843,34 @@ restart:
  * returns NULL or the traversal stops being of interest.
  *
  * If the tree is changed during traversal, results of further calls to
- * rb_iterate are unspecified.
+ * rb_iterate are unspecified.  Multiple concurrent iterators on the same
+ * tree are allowed.
  *
- * Note: this used to return a separately palloc'd iterator control struct,
- * but that's a bit pointless since the data structure is incapable of
- * supporting multiple concurrent traversals.  Now we just keep the state
- * in RBTree.
+ * The iterator state is stored in the 'iter' struct.  The caller should
+ * treat it as opaque struct.
  */
 void
-rb_begin_iterate(RBTree *rb, RBOrderControl ctrl)
+rb_begin_iterate(RBTree *rb, RBOrderControl ctrl, RBTreeIterator *iter)
 {
-       rb->cur = rb->root;
-       if (rb->cur != RBNIL)
-               rb->cur->iteratorState = InitialState;
+       /* Common initialization for all traversal orders */
+       iter->rb = rb;
+       iter->last_visited = NULL;
+       iter->is_over = (rb->root == RBNIL);
 
        switch (ctrl)
        {
                case LeftRightWalk:             /* visit left, then self, then right */
-                       rb->iterate = rb_left_right_iterator;
+                       iter->iterate = rb_left_right_iterator;
                        break;
                case RightLeftWalk:             /* visit right, then self, then left */
-                       rb->iterate = rb_right_left_iterator;
+                       iter->iterate = rb_right_left_iterator;
                        break;
                case DirectWalk:                /* visit self, then left, then right */
-                       rb->iterate = rb_direct_iterator;
+                       iter->iterate = rb_direct_iterator;
                        break;
                case InvertedWalk:              /* visit left, then right, then self */
-                       rb->iterate = rb_inverted_iterator;
+                       iter->iterate = rb_inverted_iterator;
+                       iter->next_step = NextStepBegin;
                        break;
                default:
                        elog(ERROR, "unrecognized rbtree iteration order: %d", ctrl);
@@ -864,10 +881,10 @@ rb_begin_iterate(RBTree *rb, RBOrderControl ctrl)
  * rb_iterate: return the next node in traversal order, or NULL if no more
  */
 RBNode *
-rb_iterate(RBTree *rb)
+rb_iterate(RBTreeIterator *iter)
 {
-       if (rb->cur == RBNIL)
+       if (iter->is_over)
                return NULL;
 
-       return rb->iterate(rb);
+       return iter->iterate(iter);
 }

diff --git a/src/include/access/gin_private.h b/src/include/access/gin_private.h
index 68cfe0c1ac0cf349da6f98801a3aa513c323d8bb..bf589ab7a1aedd068c7ea85f045e46474c4229f8 100644 (file)
--- a/src/include/access/gin_private.h
+++ b/src/include/access/gin_private.h
@@ -919,6 +919,7 @@ typedef struct
        GinEntryAccumulator *entryallocator;
        uint32          eas_used;
        RBTree     *tree;
+       RBTreeIterator tree_walk;
 } BuildAccumulator;
 
 extern void ginInitBA(BuildAccumulator *accum);

diff --git a/src/include/lib/rbtree.h b/src/include/lib/rbtree.h
index 73e83c367a198732281967d54957750921b7ed27..4537c587ac203b1978108058713e81497e110335 100644 (file)
--- a/src/include/lib/rbtree.h
+++ b/src/include/lib/rbtree.h
@@ -22,7 +22,6 @@
  */
 typedef struct RBNode
 {
-       char            iteratorState;  /* workspace for iterating through tree */
        char color;                                     /* node's current color, red or black */
        struct RBNode *left;            /* left child, or RBNIL if none */
        struct RBNode *right;           /* right child, or RBNIL if none */
@@ -41,6 +40,22 @@ typedef enum RBOrderControl
        InvertedWalk                            /* postorder: left child, right child, node */
 } RBOrderControl;
 
+/*
+ * RBTreeIterator holds state while traversing a tree.  This is declared
+ * here so that callers can stack-allocate this, but must otherwise be
+ * treated as an opaque struct.
+ */
+typedef struct RBTreeIterator RBTreeIterator;
+
+struct RBTreeIterator
+{
+       RBTree     *rb;
+       RBNode     *(*iterate) (RBTreeIterator *iter);
+       RBNode     *last_visited;
+       char            next_step;
+       bool            is_over;
+};
+
 /* Support functions to be provided by caller */
 typedef int (*rb_comparator) (const RBNode *a, const RBNode *b, void *arg);
 typedef void (*rb_combiner) (RBNode *existing, const RBNode *newdata, void *arg);
@@ -60,7 +75,8 @@ extern RBNode *rb_leftmost(RBTree *rb);
 extern RBNode *rb_insert(RBTree *rb, const RBNode *data, bool *isNew);
 extern void rb_delete(RBTree *rb, RBNode *node);
 
-extern void rb_begin_iterate(RBTree *rb, RBOrderControl ctrl);
-extern RBNode *rb_iterate(RBTree *rb);
+extern void rb_begin_iterate(RBTree *rb, RBOrderControl ctrl,
+                                RBTreeIterator *iter);
+extern RBNode *rb_iterate(RBTreeIterator *iter);
 
 #endif   /* RBTREE_H */