This adds strong typing via std::variant to extracting internals via …

…transaction->GetHandle()

Returning either Transaction * or ReadOptions * seems dubious, the intent of this commit is to retain 1-to-1 compatibility while adding strong typing.

nano/store/CMakeLists.txt

@@ -47,6 +47,7 @@ add_library(
   rocksdb/rocksdb.hpp
   rocksdb/iterator.hpp
   rocksdb/transaction_impl.hpp
+  rocksdb/utility.hpp
   rocksdb/version.hpp
   tables.hpp
   transaction.hpp
@@ -91,6 +92,7 @@ add_library(
   rocksdb/rep_weight.cpp
   rocksdb/rocksdb.cpp
   rocksdb/transaction.cpp
+  rocksdb/utility.cpp
   rocksdb/version.cpp
   transaction.cpp
   version.cpp

nano/store/rocksdb/iterator.hpp

@@ -3,6 +3,7 @@
 #include <nano/store/component.hpp>
 #include <nano/store/iterator.hpp>
 #include <nano/store/rocksdb/db_val.hpp>
+#include <nano/store/rocksdb/utility.hpp>
 #include <nano/store/transaction.hpp>
 
 #include <rocksdb/db.h>
@@ -11,20 +12,6 @@
 #include <rocksdb/slice.h>
 #include <rocksdb/utilities/transaction.h>
 
-namespace
-{
-inline bool is_read (nano::store::transaction const & transaction_a)
-{
-	return (dynamic_cast<nano::store::read_transaction const *> (&transaction_a) != nullptr);
-}
-
-inline rocksdb::ReadOptions & snapshot_options (nano::store::transaction const & transaction_a)
-{
-	debug_assert (is_read (transaction_a));
-	return *static_cast<rocksdb::ReadOptions *> (transaction_a.get_handle ());
-}
-}
-
 namespace nano::store::rocksdb
 {
 template <typename T, typename U>
@@ -36,19 +23,27 @@ class iterator : public iterator_impl<T, U>
 	iterator (::rocksdb::DB * db, transaction const & transaction_a, ::rocksdb::ColumnFamilyHandle * handle_a, db_val const * val_a, bool const direction_asc) :
 		iterator_impl<T, U> (transaction_a)
 	{
-		// Don't fill the block cache for any blocks read as a result of an iterator
-		if (is_read (transaction_a))
-		{
-			auto read_options = snapshot_options (transaction_a);
-			read_options.fill_cache = false;
-			cursor.reset (db->NewIterator (read_options, handle_a));
-		}
-		else
-		{
-			::rocksdb::ReadOptions ropts;
-			ropts.fill_cache = false;
-			cursor.reset (tx (transaction_a)->GetIterator (ropts, handle_a));
-		}
+		auto internals = rocksdb::tx (transaction_a);
+		auto iterator = std::visit ([&] (auto && ptr) {
+			using V = std::remove_cvref_t<decltype (ptr)>;
+			if constexpr (std::is_same_v<V, ::rocksdb::Transaction *>)
+			{
+				::rocksdb::ReadOptions ropts;
+				ropts.fill_cache = false;
+				return ptr->GetIterator (ropts, handle_a);
+			}
+			else if constexpr (std::is_same_v<V, ::rocksdb::ReadOptions *>)
+			{
+				ptr->fill_cache = false;
+				return db->NewIterator (*ptr, handle_a);
+			}
+			else
+			{
+				static_assert (sizeof (V) == 0, "Missing variant handler for type V");
+			}
+		},
+		internals);
+		cursor.reset (iterator);
 
 		if (val_a)
 		{
@@ -197,11 +192,5 @@ class iterator : public iterator_impl<T, U>
 
 	std::unique_ptr<::rocksdb::Iterator> cursor;
 	std::pair<nano::store::rocksdb::db_val, nano::store::rocksdb::db_val> current;
-
-private:
-	::rocksdb::Transaction * tx (store::transaction const & transaction_a) const
-	{
-		return static_cast<::rocksdb::Transaction *> (transaction_a.get_handle ());
-	}
 };
 }

nano/store/rocksdb/rocksdb.cpp

@@ -3,6 +3,7 @@
 #include <nano/store/rocksdb/iterator.hpp>
 #include <nano/store/rocksdb/rocksdb.hpp>
 #include <nano/store/rocksdb/transaction_impl.hpp>
+#include <nano/store/rocksdb/utility.hpp>
 #include <nano/store/version.hpp>
 
 #include <boost/format.hpp>
@@ -499,19 +500,27 @@ rocksdb::ColumnFamilyHandle * nano::store::rocksdb::component::table_to_column_f
 bool nano::store::rocksdb::component::exists (store::transaction const & transaction_a, tables table_a, nano::store::rocksdb::db_val const & key_a) const
 {
 	::rocksdb::PinnableSlice slice;
-	::rocksdb::Status status;
-	if (is_read (transaction_a))
-	{
-		status = db->Get (snapshot_options (transaction_a), table_to_column_family (table_a), key_a, &slice);
-	}
-	else
-	{
-		::rocksdb::ReadOptions options;
-		options.fill_cache = false;
-		status = tx (transaction_a)->Get (options, table_to_column_family (table_a), key_a, &slice);
-	}
+	auto internals = rocksdb::tx (transaction_a);
+	auto status = std::visit ([&] (auto && ptr) {
+		using V = std::remove_cvref_t<decltype (ptr)>;
+		if constexpr (std::is_same_v<V, ::rocksdb::Transaction *>)
+		{
+			::rocksdb::ReadOptions options;
+			options.fill_cache = false;
+			return ptr->Get (options, table_to_column_family (table_a), key_a, &slice);
+		}
+		else if constexpr (std::is_same_v<V, ::rocksdb::ReadOptions *>)
+		{
+			return db->Get (*ptr, table_to_column_family (table_a), key_a, &slice);
+		}
+		else
+		{
+			static_assert (sizeof (V) == 0, "Missing variant handler for type V");
+		}
+	},
+	internals);
 
-	return (status.ok ());
+	return status.ok ();
 }
 
 int nano::store::rocksdb::component::del (store::write_transaction const & transaction_a, tables table_a, nano::store::rocksdb::db_val const & key_a)
@@ -520,7 +529,7 @@ int nano::store::rocksdb::component::del (store::write_transaction const & trans
 	// RocksDB does not report not_found status, it is a pre-condition that the key exists
 	debug_assert (exists (transaction_a, table_a, key_a));
 	flush_tombstones_check (table_a);
-	return tx (transaction_a)->Delete (table_to_column_family (table_a), key_a).code ();
+	return std::get<::rocksdb::Transaction *> (rocksdb::tx (transaction_a))->Delete (table_to_column_family (table_a), key_a).code ();
 }
 
 void nano::store::rocksdb::component::flush_tombstones_check (tables table_a)
@@ -543,26 +552,28 @@ void nano::store::rocksdb::component::flush_table (nano::tables table_a)
 	db->Flush (::rocksdb::FlushOptions{}, table_to_column_family (table_a));
 }
 
-rocksdb::Transaction * nano::store::rocksdb::component::tx (store::transaction const & transaction_a) const
-{
-	debug_assert (!is_read (transaction_a));
-	return static_cast<::rocksdb::Transaction *> (transaction_a.get_handle ());
-}
-
 int nano::store::rocksdb::component::get (store::transaction const & transaction_a, tables table_a, nano::store::rocksdb::db_val const & key_a, nano::store::rocksdb::db_val & value_a) const
 {
 	::rocksdb::ReadOptions options;
 	::rocksdb::PinnableSlice slice;
 	auto handle = table_to_column_family (table_a);
-	::rocksdb::Status status;
-	if (is_read (transaction_a))
-	{
-		status = db->Get (snapshot_options (transaction_a), handle, key_a, &slice);
-	}
-	else
-	{
-		status = tx (transaction_a)->Get (options, handle, key_a, &slice);
-	}
+	auto internals = rocksdb::tx (transaction_a);
+	auto status = std::visit ([&] (auto && ptr) {
+		using V = std::remove_cvref_t<decltype (ptr)>;
+		if constexpr (std::is_same_v<V, ::rocksdb::Transaction *>)
+		{
+			return ptr->Get (options, handle, key_a, &slice);
+		}
+		else if constexpr (std::is_same_v<V, ::rocksdb::ReadOptions *>)
+		{
+			return db->Get (*ptr, handle, key_a, &slice);
+		}
+		else
+		{
+			static_assert (sizeof (V) == 0, "Missing variant handler for type V");
+		}
+	},
+	internals);
 
 	if (status.ok ())
 	{
@@ -576,8 +587,7 @@ int nano::store::rocksdb::component::get (store::transaction const & transaction
 int nano::store::rocksdb::component::put (store::write_transaction const & transaction_a, tables table_a, nano::store::rocksdb::db_val const & key_a, nano::store::rocksdb::db_val const & value_a)
 {
 	debug_assert (transaction_a.contains (table_a));
-	auto txn = tx (transaction_a);
-	return txn->Put (table_to_column_family (table_a), key_a, value_a).code ();
+	return std::get<::rocksdb::Transaction *> (rocksdb::tx (transaction_a))->Put (table_to_column_family (table_a), key_a, value_a).code ();
 }
 
 bool nano::store::rocksdb::component::not_found (int status) const

nano/store/rocksdb/rocksdb.hpp

@@ -122,7 +122,6 @@ class component : public nano::store::component
 	std::unordered_map<nano::tables, tombstone_info> tombstone_map;
 	std::unordered_map<char const *, nano::tables> cf_name_table_map;
 
-	::rocksdb::Transaction * tx (store::transaction const & transaction_a) const;
 	std::vector<nano::tables> all_tables () const;
 
 	bool not_found (int status) const override;

nano/store/rocksdb/utility.cpp

@@ -0,0 +1,11 @@
+#include <nano/store/rocksdb/transaction_impl.hpp>
+#include <nano/store/rocksdb/utility.hpp>
+
+auto nano::store::rocksdb::tx (store::transaction const & transaction_a) -> std::variant<::rocksdb::Transaction *, ::rocksdb::ReadOptions *>
+{
+	if (dynamic_cast<nano::store::read_transaction const *> (&transaction_a) != nullptr)
+	{
+		return static_cast<::rocksdb::ReadOptions *> (transaction_a.get_handle ());
+	}
+	return static_cast<::rocksdb::Transaction *> (transaction_a.get_handle ());
+}

nano/store/rocksdb/utility.hpp

@@ -0,0 +1,15 @@
+#pragma once
+
+#include <variant>
+
+#include <rocksdb/utilities/transaction_db.h>
+
+namespace nano::store
+{
+class transaction;
+}
+
+namespace nano::store::rocksdb
+{
+auto tx (store::transaction const & transaction_a) -> std::variant<::rocksdb::Transaction *, ::rocksdb::ReadOptions *>;
+}