Hey Alexander, > But I think we can support strict inequalities too (e.g. less and greater without equals). Could you please make it a bool argument equal_matches?
Sure, an argument is a good idea to keep the code shorter. > Could you please extract this change as a separate patch. Done! On Thu, 30 Jun 2022 at 14:34, Alexander Korotkov <aekorot...@gmail.com> wrote: > Hi, Steve! > > Thank you for working on this. > > On Thu, Jun 30, 2022 at 7:51 PM Steve Chavez <st...@supabase.io> wrote: > > Currently the red-black tree implementation only has an equality search. > Other extensions might need other comparison searches, like less-or-equal > or greater-or-equal. For example OrioleDB defines a greater-or-equal search > on its postgres fork: > > > > > https://github.com/orioledb/postgres/blob/4c18ae94c20e3e95c374b9947f1ace7d1d6497a1/src/backend/lib/rbtree.c#L164-L186 > > > > So I thought this might be valuable to have in core. I've added > less-or-equal and greater-or-equal searches functions plus tests in the > test_rbtree module. I can add the remaining less/great searches if this is > deemed worth it. > > Looks good. But I think we can support strict inequalities too (e.g. > less and greater without equals). Could you please make it a bool > argument equal_matches? > > > Also I refactored the sentinel used in the rbtree.c to use C99 > designators. > > Could you please extract this change as a separate patch. > > ------ > Regards, > Alexander Korotkov >
From 8046463e2c98fb4c51ec05740cee130bdf2ad533 Mon Sep 17 00:00:00 2001 From: steve-chavez <stevechavez...@gmail.com> Date: Tue, 28 Jun 2022 23:46:38 -0500 Subject: [PATCH 1/2] Change rbtree sentinel to C99 designator --- src/backend/lib/rbtree.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/backend/lib/rbtree.c b/src/backend/lib/rbtree.c index a9981dbada..e1cc4110bd 100644 --- a/src/backend/lib/rbtree.c +++ b/src/backend/lib/rbtree.c @@ -62,7 +62,7 @@ struct RBTree static RBTNode sentinel = { - RBTBLACK, RBTNIL, RBTNIL, NULL + .color = RBTBLACK,.left = RBTNIL,.right = RBTNIL,.parent = NULL }; -- 2.34.1
From 5d49ddd764b05083e1c276df0b50d43ec6896931 Mon Sep 17 00:00:00 2001 From: steve-chavez <stevechavez...@gmail.com> Date: Wed, 29 Jun 2022 16:28:02 -0500 Subject: [PATCH 2/2] Add rbtree missing comparison searches * Add find great/less or equal by adding an argument --- src/backend/lib/rbtree.c | 61 +++++++++++++ src/include/lib/rbtree.h | 2 + src/test/modules/test_rbtree/test_rbtree.c | 101 +++++++++++++++++++++ 3 files changed, 164 insertions(+) diff --git a/src/backend/lib/rbtree.c b/src/backend/lib/rbtree.c index e1cc4110bd..92f6b99164 100644 --- a/src/backend/lib/rbtree.c +++ b/src/backend/lib/rbtree.c @@ -161,6 +161,67 @@ rbt_find(RBTree *rbt, const RBTNode *data) return NULL; } +/* + * rbt_find_great: search for a greater value in an RBTree + * + * If equal_match is true, this will be a great or equal search. + * + * Returns the matching tree entry, or NULL if no match is found. + */ +RBTNode * +rbt_find_great(RBTree *rbt, const RBTNode *data, bool equal_match) +{ + RBTNode *node = rbt->root; + RBTNode *greater = NULL; + + while (node != RBTNIL) + { + int cmp = rbt->comparator(data, node, rbt->arg); + + if (equal_match && cmp == 0) + return node; + else if (cmp < 0) + { + greater = node; + node = node->left; + } + else + node = node->right; + } + + return greater; +} + +/* + * rbt_find_less: search for a lesser value in an RBTree + * + * If equal_match is true, this will be a less or equal search. + * + * Returns the matching tree entry, or NULL if no match is found. + */ +RBTNode * +rbt_find_less(RBTree *rbt, const RBTNode *data, bool equal_match) +{ + RBTNode *node = rbt->root; + RBTNode *lesser = NULL; + + while (node != RBTNIL) + { + int cmp = rbt->comparator(data, node, rbt->arg); + + if (equal_match && cmp == 0) + return node; + else if (cmp > 0){ + lesser = node; + node = node->right; + } + else + node = node->left; + } + + return lesser; +} + /* * rbt_leftmost: fetch the leftmost (smallest-valued) tree node. * Returns NULL if tree is empty. diff --git a/src/include/lib/rbtree.h b/src/include/lib/rbtree.h index 580a3e3439..13cf68415e 100644 --- a/src/include/lib/rbtree.h +++ b/src/include/lib/rbtree.h @@ -67,6 +67,8 @@ extern RBTree *rbt_create(Size node_size, void *arg); extern RBTNode *rbt_find(RBTree *rbt, const RBTNode *data); +extern RBTNode *rbt_find_great(RBTree *rbt, const RBTNode *data, bool equal_match); +extern RBTNode *rbt_find_less(RBTree *rbt, const RBTNode *data, bool equal_match); extern RBTNode *rbt_leftmost(RBTree *rbt); extern RBTNode *rbt_insert(RBTree *rbt, const RBTNode *data, bool *isNew); diff --git a/src/test/modules/test_rbtree/test_rbtree.c b/src/test/modules/test_rbtree/test_rbtree.c index 7cb38759a2..c17d524e01 100644 --- a/src/test/modules/test_rbtree/test_rbtree.c +++ b/src/test/modules/test_rbtree/test_rbtree.c @@ -278,6 +278,105 @@ testfind(int size) } } +/* + * Check the correctness of the rbt_find_great function by searching for + * an equal key and all of the greater keys. + */ +static void +testfindgte(int size) +{ + RBTree *tree = create_int_rbtree(); + + /* + * Using the size as the random key to search wouldn't allow us to get at + * least one greater match, so we do size - 1 + */ + int randomKey = pg_prng_uint64_range(&pg_global_prng_state, 0, size - 1); + IntRBTreeNode searchNode = {.key = randomKey}; + IntRBTreeNode *eqNode; + IntRBTreeNode *gtNode; + + /* Insert natural numbers */ + rbt_populate(tree, size, 1); + + /* + * Since the search key is included in the naturals of the tree, we're + * sure to find an equal match + */ + eqNode = (IntRBTreeNode *) rbt_find_great(tree, (RBTNode *) &searchNode, true); + + if (eqNode == NULL) + elog(ERROR, "key was not found"); + + /* ensure we find an equal match */ + if (!(eqNode->key == searchNode.key)) + elog(ERROR, "rbt_find_great operation in rbtree didn't find an equal key"); + + /* Find the rest of the naturals greater than the search key */ + for (int i = 1; i < size - searchNode.key; i++) + { + // switch equal_match to false so we only find greater matches now + gtNode = (IntRBTreeNode *) rbt_find_great(tree, (RBTNode *) &searchNode, false); + + /* ensure we find a greater match */ + if (!(gtNode->key > searchNode.key)) + elog(ERROR, "rbt_find_great operation in rbtree didn't find a greater key"); + + /* delete the previous match to find a greater one */ + rbt_delete(tree, (RBTNode *) gtNode); + } +} + +/* + * Check the correctness of the rbt_find_less operation by searching for + * an equal key and all of the lesser keys. + */ +static void +testfindlte(int size) +{ + RBTree *tree = create_int_rbtree(); + + /* + * Using 0 as the random key to search wouldn't allow us to get at least + * one lesser match, so we start from 1 + */ + int randomKey = pg_prng_uint64_range(&pg_global_prng_state, 1, size); + IntRBTreeNode searchNode = {.key = randomKey}; + IntRBTreeNode *eqNode; + IntRBTreeNode *ltNode; + + /* Insert natural numbers */ + rbt_populate(tree, size, 1); + + /* + * Since the search key is included in the naturals of the tree, we're + * sure to find an equal match + */ + eqNode = (IntRBTreeNode *) rbt_find_less(tree, (RBTNode *) &searchNode, true); + + if (eqNode == NULL) + elog(ERROR, "key was not found"); + + /* ensure we find an equal match */ + if (!(eqNode->key == searchNode.key)){ + elog(ERROR, "rbt_find_less operation in rbtree didn't find an equal key"); + } + + /* Find the rest of the naturals lesser than the search key */ + for (int i = searchNode.key; i > 0; i--) + { + // switch equal_match to false so we only find lesser matches now + ltNode = (IntRBTreeNode *) rbt_find_less(tree, (RBTNode *) &searchNode, false); + + /* ensure we find a lesser match */ + if (!(ltNode->key < searchNode.key)) + elog(ERROR, "rbt_find_less operation in rbtree didn't find a lesser key"); + + /* delete the previous match to find a lesser one */ + rbt_delete(tree, (RBTNode *) ltNode); + } +} + /* * Check the correctness of the rbt_leftmost operation. * This operation should always return the smallest element of the tree. @@ -408,6 +507,8 @@ test_rb_tree(PG_FUNCTION_ARGS) testleftright(size); testrightleft(size); testfind(size); + testfindgte(size); + testfindlte(size); testleftmost(size); testdelete(size, Max(size / 10, 1)); PG_RETURN_VOID(); -- 2.34.1