wgtmac commented on code in PR #889:
URL: https://github.com/apache/iceberg-cpp/pull/889#discussion_r3793435071
##########
src/iceberg/arrow/s3/s3_properties.h:
##########
@@ -22,6 +22,8 @@
/// \file iceberg/arrow/s3/s3_properties.h
/// \brief Define S3 configuration property keys.
+#include <algorithm>
+#include <array>
#include <string_view>
Review Comment:
std::ranges::contains is declared in <algorithm>, which this header already
includes.
##########
src/iceberg/file_io_registry.cc:
##########
@@ -19,56 +19,98 @@
#include "iceberg/file_io_registry.h"
+#include <algorithm>
#include <mutex>
+#include <ranges>
+#include <string>
#include <utility>
+#include <vector>
-#include "iceberg/resolving_file_io.h"
+#include "iceberg/util/macros.h"
+#include "iceberg/util/string_util.h"
namespace iceberg {
namespace {
struct RegistryState {
- std::mutex mutex;
- std::unordered_map<std::string, FileIORegistry::Factory> registry;
+ struct Entry {
+ std::string name;
+ FileIORegistry::Factory factory;
+ };
- RegistryState() {
- // Always available: the scheme-resolving FileIO lives in the core library.
- registry[std::string(FileIORegistry::kResolvingFileIO)] =
- [](const std::unordered_map<std::string, std::string>& properties)
- -> Result<std::unique_ptr<FileIO>> {
- return std::make_unique<ResolvingFileIO>(properties);
- };
- }
+ std::mutex mutex;
+ std::vector<Entry> registrations;
};
RegistryState& State() {
static RegistryState state;
return state;
}
+// Copy entries so user callbacks run outside the registry lock.
+std::vector<RegistryState::Entry> SnapshotEntries() {
+ auto& state = State();
+ std::lock_guard lock(state.mutex);
+ return state.registrations;
+}
+
+std::string FormatNames(const std::vector<RegistryState::Entry>& entries) {
+ std::string result;
+ for (const auto& entry : entries) {
+ if (!result.empty()) {
+ result += ", ";
+ }
+ result += entry.name;
+ }
+ return result.empty() ? "(none registered)" : result;
+}
+
} // namespace
-void FileIORegistry::Register(const std::string& name, Factory factory) {
+void FileIORegistry::Register(std::string name, Factory factory) {
auto& state = State();
std::lock_guard lock(state.mutex);
- state.registry[name] = std::move(factory);
+ std::erase_if(state.registrations, [&name](const RegistryState::Entry&
entry) {
+ return entry.name == name;
+ });
+ state.registrations.emplace_back(std::move(name), std::move(factory));
}
Review Comment:
This is valid in C++23; parenthesized aggregate initialization has been
supported since C++20, and all CI compilers build it successfully.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
