Tidy up C++17 rollback

cpp/arcticdb/CMakeLists.txt

@@ -486,7 +486,7 @@ set(arcticdb_srcs
         version/symbol_list.cpp
         version/version_map_batch_methods.cpp
         storage/s3/ec2_utils.cpp
-)
+        storage/lmdb/lmdb.hpp)
 
 if(${ARCTICDB_INCLUDE_ROCKSDB})
     list (APPEND arcticdb_srcs

cpp/arcticdb/column_store/column.hpp

@@ -677,6 +677,7 @@ class Column {
     template <
             typename input_tdt,
             typename functor>
+    // requires std::is_invocable_r_v<void, functor, typename input_tdt::DataTypeTag::raw_type> //TODO reinstate with C++20
     static void for_each(const Column& input_column, functor&& f) {
         auto input_data = input_column.data();
         std::for_each(input_data.cbegin<input_tdt>(), input_data.cend<input_tdt>(), std::forward<functor>(f));
@@ -685,6 +686,7 @@ class Column {
     template <
             typename input_tdt,
             typename functor>
+    //requires std::is_invocable_r_v<void, functor, typename ColumnData::Enumeration<typename input_tdt::DataTypeTag::raw_type>>
     static void for_each_enumerated(const Column& input_column, functor&& f) {
         auto input_data = input_column.data();
         if (input_column.is_sparse()) {
@@ -700,6 +702,7 @@ class Column {
         typename input_tdt,
         typename output_tdt,
         typename functor>
+    //  requires std::is_invocable_r_v<typename output_tdt::DataTypeTag::raw_type, functor, typename input_tdt::DataTypeTag::raw_type>
     static void transform(const Column& input_column, Column& output_column, functor&& f) {
         auto input_data = input_column.data();
         initialise_output_column(input_column, output_column);
@@ -717,6 +720,11 @@ class Column {
             typename right_input_tdt,
             typename output_tdt,
             typename functor>
+    /*requires std::is_invocable_r_v<
+            typename output_tdt::DataTypeTag::raw_type,
+            functor,
+            typename left_input_tdt::DataTypeTag::raw_type,
+            typename right_input_tdt::DataTypeTag::raw_type>*/
     static void transform(const Column& left_input_column,
                           const Column& right_input_column,
                           Column& output_column,
@@ -776,6 +784,7 @@ class Column {
     template <
             typename input_tdt,
             typename functor>
+    // std::predicate<typename input_tdt::DataTypeTag::raw_type> functor>
     static void transform(const Column& input_column,
                           util::BitSet& output_bitset,
                           bool sparse_missing_value_output,
@@ -798,6 +807,7 @@ class Column {
     template <
             typename left_input_tdt,
             typename right_input_tdt,
+            // std::relation<typename left_input_tdt::DataTypeTag::raw_type, typename right_input_tdt::DataTypeTag::raw_type> functor>
             typename functor>
     static void transform(const Column& left_input_column,
                           const Column& right_input_column,

cpp/arcticdb/log/log.cpp

@@ -72,7 +72,7 @@ struct Loggers::Impl
 
 
 constexpr auto get_default_log_level() {
-    return spdlog::level::info;
+    return spdlog::level::trace;
 }
 
 spdlog::logger &storage() {

cpp/arcticdb/processing/test/benchmark_clause.cpp

@@ -170,9 +170,9 @@ void BM_hash_grouping_string(benchmark::State& state) {
 BENCHMARK(BM_merge_interleaved)->Args({10'000, 100});
 BENCHMARK(BM_merge_ordered)->Args({10'000, 100});
 
-//BENCHMARK(BM_hash_grouping_int<int8_t>)->Args({100'000, 10, 2})->Args({100'000, 100'000, 2});
-//BENCHMARK(BM_hash_grouping_int<int16_t>)->Args({100'000, 10, 2})->Args({100'000, 100'000, 2});
-//BENCHMARK(BM_hash_grouping_int<int32_t>)->Args({100'000, 10, 2})->Args({100'000, 100'000, 2});
-//BENCHMARK(BM_hash_grouping_int<int64_t>)->Args({100'000, 10, 2})->Args({100'000, 100'000, 2});
+BENCHMARK(BM_hash_grouping_int<int8_t>)->Args({100'000, 10, 2})->Args({100'000, 100'000, 2});
+BENCHMARK(BM_hash_grouping_int<int16_t>)->Args({100'000, 10, 2})->Args({100'000, 100'000, 2});
+BENCHMARK(BM_hash_grouping_int<int32_t>)->Args({100'000, 10, 2})->Args({100'000, 100'000, 2});
+BENCHMARK(BM_hash_grouping_int<int64_t>)->Args({100'000, 10, 2})->Args({100'000, 100'000, 2});
 
-//BENCHMARK(BM_hash_grouping_string)->Args({100'000, 10, 2, 10})->Args({100'000, 100'000, 2, 10})->Args({100'000, 10, 2, 100})->Args({100'000, 100'000, 2, 100});
+BENCHMARK(BM_hash_grouping_string)->Args({100'000, 10, 2, 10})->Args({100'000, 100'000, 2, 10})->Args({100'000, 10, 2, 100})->Args({100'000, 100'000, 2, 100});

cpp/arcticdb/storage/lmdb/lmdb.hpp

@@ -0,0 +1,14 @@
+#pragma once
+
+// LMDB++ is using `std::is_pod` in `lmdb++.h`, which is deprecated as of C++20.
+// See: https://github.com/drycpp/lmdbxx/blob/0b43ca87d8cfabba392dfe884eb1edb83874de02/lmdb%2B%2B.h#L1068
+// See: https://en.cppreference.com/w/cpp/types/is_pod
+// This suppresses the warning.
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
+#ifdef ARCTICDB_USING_CONDA
+#include <lmdb++.h>
+#else
+#include <third_party/lmdbcxx/lmdb++.h>
+#endif
+#pragma GCC diagnostic pop

cpp/arcticdb/storage/lmdb/lmdb_client_wrapper.hpp

@@ -12,18 +12,10 @@
 #include <arcticdb/entity/variant_key.hpp>
 #include <arcticdb/storage/storage_utils.hpp>
 
-// LMDB++ is using `std::is_pod` in `lmdb++.h`, which is deprecated as of C++20.
-// See: https://github.com/drycpp/lmdbxx/blob/0b43ca87d8cfabba392dfe884eb1edb83874de02/lmdb%2B%2B.h#L1068
-// See: https://en.cppreference.com/w/cpp/types/is_pod
-// This suppresses the warning.
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
-#ifdef ARCTICDB_USING_CONDA
-#include <lmdb++.h>
-#else
-#include <third_party/lmdbcxx/lmdb++.h>
-#endif
-#pragma GCC diagnostic pop
+namespace lmdb {
+class txn;
+class dbi;
+}
 
 
 namespace arcticdb::storage::lmdb {

cpp/arcticdb/storage/lmdb/lmdb_mock_client.cpp

@@ -11,6 +11,7 @@
 #include <arcticdb/entity/atom_key.hpp>
 #include <arcticdb/entity/serialized_key.hpp>
 #include <arcticdb/util/string_utils.hpp>
+#include <arcticdb/storage/lmdb/lmdb.hpp>
 
 
 namespace arcticdb::storage::lmdb {

cpp/arcticdb/storage/lmdb/lmdb_real_client.cpp

@@ -11,7 +11,7 @@
 #include <arcticdb/entity/variant_key.hpp>
 #include <arcticdb/storage/lmdb/lmdb_real_client.hpp>
 #include <arcticdb/storage/storage_utils.hpp>
-
+#include <arcticdb/storage/lmdb/lmdb.hpp>
 
 namespace arcticdb::storage::lmdb {
 

cpp/arcticdb/storage/lmdb/lmdb_storage.cpp

@@ -18,14 +18,13 @@
 #include <arcticdb/storage/open_mode.hpp>
 #include <arcticdb/util/format_bytes.hpp>
 #include <arcticdb/util/preconditions.hpp>
-#include <arcticdb/util/pb_util.hpp>
 #include <arcticdb/entity/serialized_key.hpp>
 #include <arcticdb/storage/storage.hpp>
 #include <arcticdb/storage/storage_options.hpp>
-#include <arcticdb/storage/storage_utils.hpp>
 
 #include <google/protobuf/io/zero_copy_stream_impl_lite.h>
 #include <folly/gen/Base.h>
+#include <arcticdb/storage/lmdb/lmdb.hpp>
 
 namespace arcticdb::storage::lmdb {
 
@@ -49,14 +48,23 @@ void raise_lmdb_exception(const ::lmdb::error& e) {
     raise<ErrorCode::E_UNEXPECTED_LMDB_ERROR>(fmt::format("Unexpected LMDB Error: LMDBError#{}: {}", error_code, e.what()));
 }
 
+::lmdb::env& LmdbStorage::env() {
+    if (!env_) {
+        raise<ErrorCode::E_UNEXPECTED_LMDB_ERROR>("Unexpected LMDB Error: Invalid operation: LMDB environment has been removed. "
+                                                  "Possibly because the library has been deleted");
+    }
+    return *env_;
+}
+
+
 void LmdbStorage::do_write_internal(Composite<KeySegmentPair>&& kvs, ::lmdb::txn& txn) {
     auto fmt_db = [](auto &&kv) { return kv.key_type(); };
 
     (fg::from(kvs.as_range()) | fg::move | fg::groupBy(fmt_db)).foreach([&](auto &&group) {
         auto db_name = fmt::format("{}", group.key());
 
         ARCTICDB_SUBSAMPLE(LmdbStorageOpenDb, 0)
-        ::lmdb::dbi& dbi = dbi_by_key_type_.at(db_name);
+        ::lmdb::dbi& dbi = *dbi_by_key_type_.at(db_name);
 
         ARCTICDB_SUBSAMPLE(LmdbStorageWriteValues, 0)
         for (auto &kv : group.values()) {
@@ -117,7 +125,7 @@ void LmdbStorage::do_read(Composite<VariantKey>&& ks, const ReadVisitor& visitor
     (fg::from(ks.as_range()) | fg::move | fg::groupBy(fmt_db)).foreach([&](auto &&group) {
         auto db_name = fmt::format("{}", group.key());
         ARCTICDB_SUBSAMPLE(LmdbStorageOpenDb, 0)
-        ::lmdb::dbi& dbi = dbi_by_key_type_.at(db_name);
+        ::lmdb::dbi& dbi = *dbi_by_key_type_.at(db_name);
         for (auto &k : group.values()) {
             auto stored_key = to_serialized_key(k);
             try {
@@ -155,7 +163,7 @@ bool LmdbStorage::do_key_exists(const VariantKey&key) {
     ARCTICDB_SUBSAMPLE(LmdbStorageOpenDb, 0)
 
     try {
-        ::lmdb::dbi& dbi = dbi_by_key_type_.at(db_name);
+        ::lmdb::dbi& dbi = *dbi_by_key_type_.at(db_name);
         auto stored_key = to_serialized_key(key);
         return lmdb_client_->exists(db_name, stored_key, txn, dbi);
     } catch ([[maybe_unused]] const ::lmdb::not_found_error &ex) {
@@ -176,7 +184,7 @@ std::vector<VariantKey> LmdbStorage::do_remove_internal(Composite<VariantKey>&&
         ARCTICDB_SUBSAMPLE(LmdbStorageOpenDb, 0)
         try {
             // If no key of this type has been written before, this can fail
-            ::lmdb::dbi& dbi = dbi_by_key_type_.at(db_name);
+            ::lmdb::dbi& dbi = *dbi_by_key_type_.at(db_name);
             for (auto &k : group.values()) {
                 auto stored_key = to_serialized_key(k);
 
@@ -230,7 +238,7 @@ bool LmdbStorage::do_fast_delete() {
         auto db_name = fmt::format("{}", key_type);
         ARCTICDB_SUBSAMPLE(LmdbStorageOpenDb, 0)
         ARCTICDB_DEBUG(log::storage(), "dropping {}", db_name);
-        ::lmdb::dbi& dbi = dbi_by_key_type_.at(db_name);
+        ::lmdb::dbi& dbi = *dbi_by_key_type_.at(db_name);
         try {
             ::lmdb::dbi_drop(dtxn, dbi);
         } catch (const ::lmdb::error& ex) {
@@ -246,7 +254,7 @@ void LmdbStorage::do_iterate_type(KeyType key_type, const IterateTypeVisitor& vi
     ARCTICDB_SAMPLE(LmdbStorageItType, 0);
     auto txn = ::lmdb::txn::begin(env(), nullptr, MDB_RDONLY); // scoped abort on
     std::string type_db = fmt::format("{}", key_type);
-    ::lmdb::dbi& dbi = dbi_by_key_type_.at(type_db);
+    ::lmdb::dbi& dbi = *dbi_by_key_type_.at(type_db);
 
     try {
         auto keys = lmdb_client_->list(type_db, prefix, txn, dbi, key_type);
@@ -332,7 +340,7 @@ LmdbStorage::LmdbStorage(const LibraryPath &library_path, OpenMode mode, const C
     }
     write_mutex_ = std::make_unique<std::mutex>();
     env_ = std::make_unique<::lmdb::env>(::lmdb::env::create(conf.flags()));
-    dbi_by_key_type_ = std::unordered_map<std::string, ::lmdb::dbi>{};
+    dbi_by_key_type_ = std::unordered_map<std::string, std::unique_ptr<::lmdb::dbi>>{};
 
     fs::path root_path = conf.path().c_str();
     auto lib_path_str = library_path.to_delim_path(fs::path::preferred_separator);
@@ -381,7 +389,7 @@ LmdbStorage::LmdbStorage(const LibraryPath &library_path, OpenMode mode, const C
         arcticdb::entity::foreach_key_type([&txn, this](KeyType &&key_type) {
             std::string db_name = fmt::format("{}", key_type);
             ::lmdb::dbi dbi = ::lmdb::dbi::open(txn, db_name.data(), MDB_CREATE);
-            dbi_by_key_type_.insert(std::make_pair(std::move(db_name), std::move(dbi)));
+            dbi_by_key_type_.insert(std::make_pair(std::move(db_name), std::make_unique<::lmdb::dbi>(std::move(dbi))));
         });
     } catch (const ::lmdb::error& ex) {
         raise_lmdb_exception(ex);

cpp/arcticdb/storage/lmdb/lmdb_storage.hpp

@@ -20,6 +20,11 @@
 
 namespace fs = std::filesystem;
 
+namespace lmdb {
+class env;
+class dbi;
+}
+
 namespace arcticdb::storage::lmdb {
 
 class LmdbStorage final : public Storage {
@@ -54,13 +59,7 @@ class LmdbStorage final : public Storage {
 
     bool do_is_path_valid(const std::string_view path) const final;
 
-    ::lmdb::env& env() {
-        if (!env_) {
-            raise<ErrorCode::E_UNEXPECTED_LMDB_ERROR>("Unexpected LMDB Error: Invalid operation: LMDB environment has been removed. "
-                                                      "Possibly because the library has been deleted");
-        }
-        return *env_;
-    }
+    ::lmdb::env& env();
 
     std::string do_key_path(const VariantKey&) const final { return {}; };
 
@@ -72,7 +71,7 @@ class LmdbStorage final : public Storage {
     std::unique_ptr<std::mutex> write_mutex_;
     std::unique_ptr<::lmdb::env> env_;
 
-    std::unordered_map<std::string, ::lmdb::dbi> dbi_by_key_type_;
+    std::unordered_map<std::string, std::unique_ptr<::lmdb::dbi>> dbi_by_key_type_;
 
     std::filesystem::path lib_dir_;
 

cpp/arcticdb/storage/mongo/mongo_mock_client.cpp

@@ -5,7 +5,6 @@
  * As of the Change Date specified in that file, in accordance with the Business Source License, use of this software will be governed by the Apache License, version 2.0.
  */
 
-#include <arcticdb/storage/storage_utils.hpp>
 #include <arcticdb/storage/mongo/mongo_client_wrapper.hpp>
 #include <arcticdb/storage/mongo/mongo_mock_client.hpp>
 #include <arcticdb/storage/object_store_utils.hpp>
@@ -14,7 +13,6 @@
 #include <mongocxx/exception/bulk_write_exception.hpp>
 #include <mongocxx/exception/query_exception.hpp>
 
-
 namespace arcticdb::storage::mongo {
 
 std::string MockMongoClient::get_failure_trigger(
@@ -45,7 +43,7 @@ MongoFailure get_failure(const std::string& message, StorageOperation operation,
         case StorageOperation::EXISTS:
             return create_failure<mongocxx::query_exception>(message, error_code);
         case StorageOperation::DELETE:
-            return create_failure<mongocxx::bulk_write_exception>(message, error_code);
+            [[fallthrough]];
         case StorageOperation::DELETE_LOCAL:
             return create_failure<mongocxx::bulk_write_exception>(message, error_code);
         case StorageOperation::LIST:
@@ -58,7 +56,7 @@ MongoFailure get_failure(const std::string& message, StorageOperation operation,
 std::optional<MongoFailure> has_failure_trigger(
         const MongoKey& key,
         StorageOperation operation) {
-    auto key_id = key.doc_key.id_string();
+    auto key_id = key.doc_key_.id_string();
     auto failure_string_for_operation = "#Failure_" + operation_to_string(operation) + "_";
     auto position = key_id.rfind(failure_string_for_operation);
     if (position == std::string::npos) {
@@ -83,8 +81,8 @@ bool matches_prefix(
         const std::string& collection_name,
         const std::string& prefix) {
 
-    return key.database_name == database_name && key.collection_name == collection_name &&
-           key.doc_key.id_string().find(prefix) == 0;
+    return key.database_name_ == database_name && key.collection_name_ == collection_name &&
+           key.doc_key_.id_string().find(prefix) == 0;
 }
 
 void throw_if_exception(MongoFailure& failure) {
@@ -109,7 +107,7 @@ bool MockMongoClient::write_segment(
         return false;
     }
 
-    mongo_contents.insert_or_assign(key, std::move(kv.segment()));
+    mongo_contents.insert_or_assign(std::move(key), std::move(kv.segment()));
     return true;
 }
 
@@ -131,7 +129,7 @@ UpdateResult MockMongoClient::update_segment(
         return {0}; // upsert is false, don't update and return 0 as modified_count
     }
 
-    mongo_contents.insert_or_assign(key, std::move(kv.segment()));
+    mongo_contents.insert_or_assign(std::move(key), std::move(kv.segment()));
     return {key_found ? 1 : 0};
 }
 
@@ -151,7 +149,7 @@ std::optional<KeySegmentPair> MockMongoClient::read_segment(
         return std::nullopt;
     }
 
-    return KeySegmentPair(std::move(mongo_key.doc_key.key), std::move(it->second));
+    return KeySegmentPair(std::move(mongo_key.doc_key_.key_), std::move(it->second));
 }
 
 DeleteResult MockMongoClient::remove_keyvalue(
@@ -203,7 +201,7 @@ std::vector<VariantKey> MockMongoClient::list_keys(
                 throw_if_exception(failure.value());
                 return {};
             }
-            output.push_back(key.first.doc_key.key);
+            output.push_back(key.first.doc_key_.key_);
         }
     }
 
@@ -216,7 +214,7 @@ void MockMongoClient::ensure_collection(std::string_view, std::string_view ) {
 
 void MockMongoClient::drop_collection(std::string database_name, std::string collection_name) {
     for (auto it = mongo_contents.begin(); it != mongo_contents.end(); ) {
-        if (it->first.database_name == database_name && it->first.collection_name == collection_name) {
+        if (it->first.database_name_ == database_name && it->first.collection_name_ == collection_name) {
             it = mongo_contents.erase(it);
         } else {
             ++it;