From 68663d0de0c5029e8b958f08848ecd4e00a018e0 Mon Sep 17 00:00:00 2001
From: Kamil Braun <kbraun@scylladb.com>
Date: Thu, 8 Oct 2020 17:15:38 +0200
Subject: [PATCH] sstables: pass ring_position to
 create_single_key_sstable_reader

instead of partition_range.

It would be best to pass `partition_key` or `decorated_key` here.
However, the implementation of this function needs a `partition_range`
to pass into `sstable_set::select`, and `partition_range` must be
constructed from `ring_position`s. We could create the `ring_position`
internally from the key but that would involve a copy which we want to
avoid.
---
 sstables/sstable_set.cc | 17 ++++++++---------
 sstables/sstable_set.hh |  2 +-
 table.cc                |  3 ++-
 3 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/sstables/sstable_set.cc b/sstables/sstable_set.cc
index 3d12d2df3f..e96ed4a4b3 100644
--- a/sstables/sstable_set.cc
+++ b/sstables/sstable_set.cc
@@ -474,11 +474,10 @@ public:
 // Filter out sstables for reader using bloom filter
 static std::vector<shared_sstable>
 filter_sstable_for_reader_by_pk(std::vector<shared_sstable>&& sstables, column_family& cf, const schema_ptr& schema,
-        const dht::partition_range& pr, const key& key) {
-    const dht::ring_position& pr_key = pr.start()->value();
+        const dht::ring_position& pos, const key& key) {
     auto sstable_has_not_key = [&, cmp = dht::ring_position_comparator(*schema)] (const shared_sstable& sst) {
-        return cmp(pr_key, sst->get_first_decorated_key()) < 0 ||
-               cmp(pr_key, sst->get_last_decorated_key()) > 0 ||
+        return cmp(pos, sst->get_first_decorated_key()) < 0 ||
+               cmp(pos, sst->get_last_decorated_key()) > 0 ||
                !sst->filter_has_key(key);
     };
     sstables.erase(boost::remove_if(sstables, sstable_has_not_key), sstables.end());
@@ -525,16 +524,16 @@ sstable_set::create_single_key_sstable_reader(
         schema_ptr schema,
         reader_permit permit,
         utils::estimated_histogram& sstable_histogram,
-        const dht::partition_range& pr,
+        const dht::ring_position& pos,
         const query::partition_slice& slice,
         const io_priority_class& pc,
         tracing::trace_state_ptr trace_state,
         streamed_mutation::forwarding fwd,
         mutation_reader::forwarding fwd_mr) const
 {
-    auto& pk = pr.start()->value().key();
+    auto& pk = pos.key();
     auto key = key::from_partition_key(*schema, *pk);
-    auto selected_sstables = filter_sstable_for_reader_by_pk(select(pr), *cf, schema, pr, key);
+    auto selected_sstables = filter_sstable_for_reader_by_pk(select({pos}), *cf, schema, pos, key);
     auto num_sstables = selected_sstables.size();
     if (!num_sstables) {
         return make_empty_flat_reader(schema, permit);
@@ -542,8 +541,8 @@ sstable_set::create_single_key_sstable_reader(
     auto readers = boost::copy_range<std::vector<flat_mutation_reader>>(
         filter_sstable_for_reader_by_ck(std::move(selected_sstables), *cf, schema, slice)
         | boost::adaptors::transformed([&] (const shared_sstable& sstable) {
-            tracing::trace(trace_state, "Reading key {} from sstable {}", pr, seastar::value_of([&sstable] { return sstable->get_filename(); }));
-            return sstable->read_row_flat(schema, permit, pr.start()->value(), slice, pc, trace_state, fwd);
+            tracing::trace(trace_state, "Reading key {} from sstable {}", pos, seastar::value_of([&sstable] { return sstable->get_filename(); }));
+            return sstable->read_row_flat(schema, permit, pos, slice, pc, trace_state, fwd);
         })
     );
 
diff --git a/sstables/sstable_set.hh b/sstables/sstable_set.hh
index 69d9d006cb..f0f5a86c73 100644
--- a/sstables/sstable_set.hh
+++ b/sstables/sstable_set.hh
@@ -111,7 +111,7 @@ public:
         schema_ptr,
         reader_permit,
         utils::estimated_histogram&,
-        const dht::partition_range&, // must be singular
+        const dht::ring_position&, // must contain a key
         const query::partition_slice&,
         const io_priority_class&,
         tracing::trace_state_ptr,
diff --git a/table.cc b/table.cc
index 97bc41f173..703879a93e 100644
--- a/table.cc
+++ b/table.cc
@@ -89,8 +89,9 @@ table::make_sstable_reader(schema_ptr s,
                     tracing::trace_state_ptr trace_state,
                     streamed_mutation::forwarding fwd,
                     mutation_reader::forwarding fwd_mr) {
+                assert(pr.is_singular() && pr.start()->value().has_key());
                 return sstables->create_single_key_sstable_reader(const_cast<column_family*>(this), std::move(s), std::move(permit),
-                        _stats.estimated_sstable_per_read, pr, slice, pc, std::move(trace_state), fwd, fwd_mr);
+                        _stats.estimated_sstable_per_read, pr.start()->value(), slice, pc, std::move(trace_state), fwd, fwd_mr);
             });
         } else {
             return mutation_source([sstables=std::move(sstables)] (