Add higher/lower search functions in gb_trees and gb_sets

[zzydxm]

Provide some common operations that is available for BSTs.

API follows java TreeSet / TreeMap

[github-actions]

CT Test Results

       2 files       92 suites   35m 17s ⏱️
2 002 tests 1 954 ✔️ 48 💤 0 ❌
2 306 runs  2 256 ✔️ 50 💤 0 ❌

Results for commit 1595acf.

♻️ This comment has been updated with latest results.

To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass.

See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally.

Artifacts

• Complete CT logs (Download Logs)
• HTML Documentation (Download HTML Docs)
• Windows Installer

// Erlang/OTP Github Action Bot

[zzydxm]

I will add java tailmap/submap/tailset/subset equivalent later in a separate PR

[zzydxm]

Do we want to be able to do gb_sets:prev(element(2, gb_sets:next(Iterator)))? If yes then we will need to change the data structure returned by iterator_from, or deprecate iterator_from and create a new set of functions.

[zzydxm]

Hi @mikpe, is there any update on this? I cannot proceed with out deciding the behavior of gb_sets:prev(element(2, gb_sets:next(Iterator))). Thanks!

[jhogberg]

@zzydxm This is stalled until we've discussed things internally, we'll get back to you as soon as we can.

[jhogberg]

After discussing this internally, we'd like the following:

To match the maps module, we'd like reverse iteration to be handled by an order argument to iterator/1 and iterator_from/2 instead of having explicit forward and backward functions. next/1 would be used for both directions with the order depending on the iterator.

We would also prefer the names smaller and larger for < and >, to match smallest/1 and largest/1.

>= and =< may have some uses but they can be trivially implemented by composing is_element/2 and smaller/2 or larger/2 in user code. They can always be added later if there is a need.

[zzydxm]

Sure I will rename these functions and fix the issues.
For iterators, order can only be ordered | reversed, not a function, right?
An iterator that can move in both direction is more powerful without much implementation complexity (iterator can still be list of TreeNodes) but single directional already satisfied our use case.

[zzydxm]

I just realized that if we only have next but not prev, then reversed iterator need to return some datatype different with iterator/1, like {reverse, iter(Element)}. Otherwise it is either more expensive to create the iterator, or we cannot distinguish whether it is forward or backward iter.

[zzydxm]

Updated the commit, I kept type spec unchanged because doing suggest way will see this error:

gb_sets.erl:511:15: type variable 'Element1' is only used once (is unbound)
%  511| -spec smaller(Element1, Set) -> none | {found, Element2} when

[jhogberg]

For iterators, order can only be ordered | reversed, not a function, right?

Yes.

I just realized that if we only have next but not prev, then reversed iterator need to return some datatype different with iterator/1, like {reverse, iter(Element)}. Otherwise it is either more expensive to create the iterator, or we cannot distinguish whether it is forward or backward iter.

Yep.

Updated the commit, I kept type spec unchanged because doing suggest way will see this error:

How about something like...?

-spec smaller(SmallerThan, Set) -> none | {found, SmallerElement} when
    SmallerThan :: term(),
    SmallerElement :: term(),
    Set :: set().

[zzydxm]

How about something like...?

Sure I can do that, but it is quite different with other typings in the module. Shall we modify them together in a later commit?

[jhogberg]

Sure I can do that, but it is quite different with other typings in the module. Shall we modify them together in a later commit?

Sure, later commit is fine.

[jhogberg]

Hi! This line needs to be updated (and for trees too?), the property tests fail to compile right now.

[zzydxm]

Sorry property test had some issue on my laptop so were skipped. It should be fixed now.

[jhogberg]

Merged, thanks for the PR!