sstables: don't expose summary binary search

There is no need to expose binary search. It can be an internal function
that is accessible for test only.

Also, in the end, the implementation of the summary version was such a simple
one, that there is no need to have a specific method for that. We can just pass
the summary entries as a parameter.

Some header file massage is needed to keep it compiling

Signed-off-by: Glauber Costa <glommer@cloudius-systems.com>

sstables/partition.cc

@@ -5,6 +5,7 @@
 #include "sstables.hh"
 #include "types.hh"
 #include "core/future-util.hh"
+#include "key.hh"
 
 #include "dht/i_partitioner.hh"
 
@@ -24,7 +25,7 @@ namespace sstables {
 // This code should work in all kinds of vectors in whose's elements is possible to aquire
 // a key view.
 template <typename T>
-int binary_search(const T& entries, const key& sk) {
+int sstable::binary_search(const T& entries, const key& sk) {
     int low = 0, mid = entries.size(), high = mid - 1, result = -1;
 
     auto& partitioner = dht::global_partitioner();
@@ -58,7 +59,7 @@ int binary_search(const T& entries, const key& sk) {
     return -mid - (result < 0 ? 1 : 2);
 }
 
-int summary::binary_search(const key& sk) {
-    return sstables::binary_search(entries, sk);
-}
+// Force generation, so we make it available outside this compilation unit without moving that
+// much code to .hh
+template int sstable::binary_search<>(const std::vector<summary_entry>& entries, const key& sk);
 }

sstables/sstables.hh

@@ -19,6 +19,7 @@
 #include "row.hh"
 
 namespace sstables {
+class key;
 
 class malformed_sstable_exception : public std::exception {
     sstring _msg;
@@ -118,7 +119,8 @@ private:
     // for iteration through all the rows.
     future<temporary_buffer<char>> data_read(uint64_t pos, size_t len);
 
-
+    template <typename T>
+    int binary_search(const T& entries, const key& sk);
 public:
     // Read one or few rows at the given byte range from the data file,
     // feeding them into the consumer. This function reads the entire given

sstables/types.hh

@@ -2,7 +2,6 @@
 
 #include "core/enum.hh"
 #include "bytes.hh"
-#include "sstables/key.hh"
 
 namespace sstables {
 
@@ -115,7 +114,6 @@ struct summary_la {
     // filled with the same data. It's too early to judge that the data is useless.
     // However, it was tested that Cassandra loads successfully a Summary file with
     // this structure removed from it. Anyway, let's pay attention to it.
-    int binary_search(const key& sk);
 };
 using summary = summary_la;
 

tests/urchin/sstable_test.cc

@@ -11,7 +11,9 @@
 #include "core/align.hh"
 #include "core/do_with.hh"
 #include "sstables/sstables.hh"
+#include "sstables/key.hh"
 #include "tests/test-utils.hh"
+#include "schema.hh"
 #include <memory>
 
 using namespace sstables;
@@ -68,6 +70,10 @@ public:
         return _sst->_components;
     }
 
+    template <typename T>
+    int binary_search(const T& entries, const key& sk) {
+        return _sst->binary_search(entries, sk);
+    }
 };
 }
 
@@ -773,7 +779,7 @@ SEASTAR_TEST_CASE(find_key_map) {
         kk.push_back(map);
 
         auto key = sstables::key::from_deeply_exploded(*s, kk);
-        BOOST_REQUIRE(summary.binary_search(key) == 0);
+        BOOST_REQUIRE(sstables::test(sstp).binary_search(summary.entries, key) == 0);
     });
 }
 
@@ -793,7 +799,7 @@ SEASTAR_TEST_CASE(find_key_set) {
         kk.push_back(set);
 
         auto key = sstables::key::from_deeply_exploded(*s, kk);
-        BOOST_REQUIRE(summary.binary_search(key) == 0);
+        BOOST_REQUIRE(sstables::test(sstp).binary_search(summary.entries, key) == 0);
     });
 }
 
@@ -813,7 +819,7 @@ SEASTAR_TEST_CASE(find_key_list) {
         kk.push_back(list);
 
         auto key = sstables::key::from_deeply_exploded(*s, kk);
-        BOOST_REQUIRE(summary.binary_search(key) == 0);
+        BOOST_REQUIRE(sstables::test(sstp).binary_search(summary.entries, key) == 0);
     });
 }
 
@@ -831,7 +837,7 @@ SEASTAR_TEST_CASE(find_key_composite) {
         kk.push_back(boost::any(b2));
 
         auto key = sstables::key::from_deeply_exploded(*s, kk);
-        BOOST_REQUIRE(summary.binary_search(key) == 0);
+        BOOST_REQUIRE(sstables::test(sstp).binary_search(summary.entries, key) == 0);
     });
 }
 
@@ -842,7 +848,7 @@ SEASTAR_TEST_CASE(all_in_place) {
         int idx = 0;
         for (auto& e: summary.entries) {
             auto key = sstables::key::from_bytes(e.key);
-            BOOST_REQUIRE(summary.binary_search(key) == idx++);
+            BOOST_REQUIRE(sstables::test(sstp).binary_search(summary.entries, key) == idx++);
         }
     });
 }
@@ -861,6 +867,6 @@ SEASTAR_TEST_CASE(not_find_key_composite_bucket0) {
 
         auto key = sstables::key::from_deeply_exploded(*s, kk);
         // (result + 1) * -1 -1 = 0
-        BOOST_REQUIRE(summary.binary_search(key) == -2);
+        BOOST_REQUIRE(sstables::test(sstp).binary_search(summary.entries, key) == -2);
     });
 }