This is an automated email from the ASF dual-hosted git repository.
SteNicholas pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/paimon-cpp.git
The following commit(s) were added to refs/heads/main by this push:
new 4027601e refactor(utils): consolidate string and option handling (#249)
4027601e is described below
commit 4027601e67f4271d5efd8ebe278efd66f2e1bee0
Author: lxy <[email protected]>
AuthorDate: Thu Aug 27 14:55:03 2026 +0800
refactor(utils): consolidate string and option handling (#249)
---
.../common/data/variant/variant_access_utils.cpp | 3 +-
src/paimon/common/types/data_type_json_parser.cpp | 6 +-
src/paimon/common/utils/options_utils.h | 20 +-
src/paimon/common/utils/options_utils_test.cpp | 12 +-
src/paimon/common/utils/string_utils.cpp | 57 ++++-
src/paimon/common/utils/string_utils.h | 31 +--
src/paimon/common/utils/string_utils_test.cpp | 32 +++
src/paimon/core/core_options.cpp | 281 +++++++++------------
.../compact/lookup_merge_tree_compact_rewriter.cpp | 11 +-
.../compact/merge_tree_compact_rewriter.cpp | 11 +-
src/paimon/core/mergetree/lookup_levels.cpp | 11 +-
.../operation/append_only_file_store_write.cpp | 8 +-
.../commit/sequence_snapshot_properties.cpp | 18 +-
.../commit/sequence_snapshot_properties_test.cpp | 2 +-
.../core/postpone/postpone_bucket_writer.cpp | 18 +-
src/paimon/core/schema/schema_validation.cpp | 2 +-
src/paimon/core/schema/schema_validation_test.cpp | 6 +
.../core/table/system/global_system_tables.cpp | 7 +-
src/paimon/format/orc/orc_format_writer.cpp | 24 +-
src/paimon/format/parquet/parquet_format_defs.h | 2 +-
src/paimon/fs/local/local_file.cpp | 2 +-
src/paimon/fs/local/local_file_test.cpp | 5 +
src/paimon/fs/s3/s3_file_system.cpp | 12 +-
src/paimon/global_index/lucene/jieba_analyzer.cpp | 6 +-
src/paimon/rest/dlf_auth.cpp | 25 +-
src/paimon/rest/rest_api.cpp | 7 +-
src/paimon/rest/rest_auth.cpp | 15 +-
src/paimon/rest/rest_catalog.cpp | 2 +-
src/paimon/rest/rest_http_client.cpp | 3 +-
src/paimon/rest/rest_util.cpp | 9 +-
30 files changed, 313 insertions(+), 335 deletions(-)
diff --git a/src/paimon/common/data/variant/variant_access_utils.cpp
b/src/paimon/common/data/variant/variant_access_utils.cpp
index 78180d1c..0238c086 100644
--- a/src/paimon/common/data/variant/variant_access_utils.cpp
+++ b/src/paimon/common/data/variant/variant_access_utils.cpp
@@ -26,6 +26,7 @@
#include "fmt/format.h"
#include "paimon/common/data/variant/variant_defs.h"
#include "paimon/common/types/data_field.h"
+#include "paimon/common/utils/string_utils.h"
namespace paimon {
@@ -61,7 +62,7 @@ std::vector<std::string> SplitDescription(const std::string&
description) {
}
bool HasAccessDescription(const std::shared_ptr<arrow::Field>& field) {
- return GetDescription(field).rfind(VariantAccessUtils::kMetadataKey, 0) ==
0;
+ return StringUtils::StartsWith(GetDescription(field),
VariantAccessUtils::kMetadataKey);
}
} // namespace
diff --git a/src/paimon/common/types/data_type_json_parser.cpp
b/src/paimon/common/types/data_type_json_parser.cpp
index e95582a1..d04e5ade 100644
--- a/src/paimon/common/types/data_type_json_parser.cpp
+++ b/src/paimon/common/types/data_type_json_parser.cpp
@@ -19,7 +19,6 @@
#include "paimon/common/types/data_type_json_parser.h"
-#include <algorithm>
#include <cctype>
#include <cstddef>
#include <cstdint>
@@ -331,10 +330,7 @@ std::vector<Token> Tokenize(const std::string& chars) {
builder.clear();
cursor = ConsumeIdentifier(chars, cursor, builder);
auto token = builder.str();
- auto normalized_token = token;
- std::transform(normalized_token.begin(),
normalized_token.end(),
- normalized_token.begin(),
- [](unsigned char c) { return std::toupper(c);
});
+ std::string normalized_token = StringUtils::ToUpperCase(token);
if (Keywords().find(normalized_token) != Keywords().end()) {
tokens.emplace_back(TokenType::KEYWORD, cursor,
normalized_token);
} else {
diff --git a/src/paimon/common/utils/options_utils.h
b/src/paimon/common/utils/options_utils.h
index c2014007..08b09ad2 100644
--- a/src/paimon/common/utils/options_utils.h
+++ b/src/paimon/common/utils/options_utils.h
@@ -89,13 +89,29 @@ class OptionsUtils {
return value.status();
}
+ static Result<std::string> GetNonEmptyValueFromMap(
+ const std::map<std::string, std::string>& key_value_map, const
std::string& key) {
+ Result<std::string> value =
GetValueFromMap<std::string>(key_value_map, key);
+ if (!value.ok()) {
+ return value.status();
+ }
+ if (value.value().empty()) {
+ return Status::Invalid(fmt::format("value for key {} must not be
empty", key));
+ }
+ return value.value();
+ }
+
/// Fetch options with specific prefix and remove prefix for key.
+ /// @param prefix Prefix used to select options and removed from the
returned keys.
+ /// @param options Options to select from.
+ /// @return Options whose keys start with and are longer than `prefix`,
with the prefix removed
+ /// from each key.
static std::map<std::string, std::string> FetchOptionsWithPrefix(
const std::string& prefix, const std::map<std::string, std::string>&
options) {
std::map<std::string, std::string> options_with_prefix;
- int64_t prefix_len = prefix.size();
+ const std::string::size_type prefix_len = prefix.size();
for (const auto& [key, value] : options) {
- if (StringUtils::StartsWith(key, prefix)) {
+ if (key.size() > prefix_len && StringUtils::StartsWith(key,
prefix)) {
options_with_prefix[key.substr(prefix_len)] = value;
}
}
diff --git a/src/paimon/common/utils/options_utils_test.cpp
b/src/paimon/common/utils/options_utils_test.cpp
index d4641184..61e52087 100644
--- a/src/paimon/common/utils/options_utils_test.cpp
+++ b/src/paimon/common/utils/options_utils_test.cpp
@@ -84,9 +84,19 @@ TEST(OptionsUtilsTest, TestGetOptionalValueFromMap) {
}
TEST(OptionsUtilsTest, TestFetchOptionsWithPrefix) {
- std::map<std::string, std::string> options = {{"key1", "value1"},
{"test.key2", "value2"}};
+ std::map<std::string, std::string> options = {
+ {"key1", "value1"}, {"test.", "empty-key"}, {"test.key2", "value2"}};
auto new_options = OptionsUtils::FetchOptionsWithPrefix("test.", options);
std::map<std::string, std::string> expected = {{"key2", "value2"}};
ASSERT_EQ(expected, new_options);
}
+
+TEST(OptionsUtilsTest, TestGetNonEmptyValueFromMap) {
+ std::map<std::string, std::string> options = {{"present", "value"},
{"empty", ""}};
+ ASSERT_OK_AND_ASSIGN(std::string value,
+ OptionsUtils::GetNonEmptyValueFromMap(options,
"present"));
+ ASSERT_EQ("value", value);
+ ASSERT_TRUE(OptionsUtils::GetNonEmptyValueFromMap(options,
"missing").status().IsNotExist());
+ ASSERT_TRUE(OptionsUtils::GetNonEmptyValueFromMap(options,
"empty").status().IsInvalid());
+}
} // namespace paimon::test
diff --git a/src/paimon/common/utils/string_utils.cpp
b/src/paimon/common/utils/string_utils.cpp
index 5b405895..869e184d 100644
--- a/src/paimon/common/utils/string_utils.cpp
+++ b/src/paimon/common/utils/string_utils.cpp
@@ -30,8 +30,28 @@
#include "paimon/status.h"
namespace paimon {
+namespace {
+
+bool IsTrimCharacter(unsigned char c) {
+ // Match the characters removed by Java String::trim for the ASCII strings
handled here.
+ return c <= 0x20;
+}
+
+char ToAsciiLower(unsigned char c) {
+ return c >= 'A' && c <= 'Z' ? static_cast<char>(c + ('a' - 'A')) :
static_cast<char>(c);
+}
+
+char ToAsciiUpper(unsigned char c) {
+ return c >= 'a' && c <= 'z' ? static_cast<char>(c - ('a' - 'A')) :
static_cast<char>(c);
+}
+
+} // namespace
+
std::string StringUtils::Replace(const std::string& text, const std::string&
search_string,
const std::string& replacement, int32_t max) {
+ if (text.empty() || search_string.empty() || max == 0) {
+ return text;
+ }
std::string str = text;
size_t pos = str.find(search_string);
int32_t count = 0;
@@ -45,6 +65,9 @@ std::string StringUtils::Replace(const std::string& text,
const std::string& sea
std::string StringUtils::ReplaceLast(const std::string& text, const
std::string& old_str,
const std::string& new_str) {
+ if (text.empty() || old_str.empty()) {
+ return text;
+ }
std::string str = text;
size_t pos = str.rfind(old_str);
if (pos != std::string::npos) {
@@ -54,7 +77,8 @@ std::string StringUtils::ReplaceLast(const std::string& text,
const std::string&
}
bool StringUtils::StartsWith(const std::string& str, const std::string&
prefix, size_t start_pos) {
- return (str.size() >= prefix.size()) && (str.compare(start_pos,
prefix.size(), prefix) == 0);
+ return start_pos <= str.size() && prefix.size() <= str.size() - start_pos
&&
+ str.compare(start_pos, prefix.size(), prefix) == 0;
}
bool StringUtils::EndsWith(const std::string& str, const std::string& suffix) {
size_t s1 = str.size();
@@ -74,26 +98,45 @@ bool StringUtils::IsNullOrWhitespaceOnly(const std::string&
str) {
}
void StringUtils::Trim(std::string* str) {
- str->erase(str->find_last_not_of(' ') + 1);
- str->erase(0, str->find_first_not_of(' '));
+ auto first = std::find_if_not(str->begin(), str->end(),
+ [](unsigned char c) { return
IsTrimCharacter(c); });
+ auto last = std::find_if_not(str->rbegin(), str->rend(), [](unsigned char
c) {
+ return IsTrimCharacter(c);
+ }).base();
+ if (first >= last) {
+ str->clear();
+ return;
+ }
+ *str = std::string(first, last);
}
std::string StringUtils::ToLowerCase(const std::string& str) {
std::string result;
result.reserve(str.length());
- std::transform(str.begin(), str.end(), std::back_inserter(result),
- [](unsigned char c) { return std::tolower(c); });
+ std::transform(str.begin(), str.end(), std::back_inserter(result),
ToAsciiLower);
return result;
}
std::string StringUtils::ToUpperCase(const std::string& str) {
std::string result;
result.reserve(str.length());
- std::transform(str.begin(), str.end(), std::back_inserter(result),
- [](unsigned char c) { return std::toupper(c); });
+ std::transform(str.begin(), str.end(), std::back_inserter(result),
ToAsciiUpper);
return result;
}
+bool StringUtils::EqualsIgnoreCase(const std::string& left, const std::string&
right) {
+ if (left.size() != right.size()) {
+ return false;
+ }
+ for (size_t i = 0; i < left.size(); ++i) {
+ if (ToAsciiLower(static_cast<unsigned char>(left[i])) !=
+ ToAsciiLower(static_cast<unsigned char>(right[i]))) {
+ return false;
+ }
+ }
+ return true;
+}
+
std::vector<std::string> StringUtils::Split(const std::string& text, const
std::string& sep_str,
bool ignore_empty) {
std::vector<std::string> vec;
diff --git a/src/paimon/common/utils/string_utils.h
b/src/paimon/common/utils/string_utils.h
index 3c0906e2..7681a8d9 100644
--- a/src/paimon/common/utils/string_utils.h
+++ b/src/paimon/common/utils/string_utils.h
@@ -50,24 +50,18 @@ class PAIMON_EXPORT StringUtils {
public:
/// Replaces all occurrences of a string within another string.
///
- /// A `null` reference passed to this method is a no-op.
- ///
/// <pre>
- /// StringUtils::Replace(null, *, *) = null
/// StringUtils::Replace("", *, *) = ""
- /// StringUtils::Replace("any", null, *) = "any"
- /// StringUtils::Replace("any", *, null) = "any"
/// StringUtils::Replace("any", "", *) = "any"
- /// StringUtils::Replace("aba", "a", null) = "aba"
/// StringUtils::Replace("aba", "a", "") = "b"
/// StringUtils::Replace("aba", "a", "z") = "zbz"
/// </pre>
///
/// @see #replace(string text, string search_string, string replacement,
int max)
- /// @param text text to search and replace in, may be null
- /// @param search_string the String to search for, may be null
- /// @param replacement the String to replace it with, may be null
- /// @return the text with any replacements processed, `null` if null
string input
+ /// @param text text to search and replace in
+ /// @param search_string the String to search for
+ /// @param replacement the String to replace it with
+ /// @return the text with any replacements processed
static std::string Replace(const std::string& text, const std::string&
search_string,
const std::string& replacement) {
return Replace(text, search_string, replacement, -1);
@@ -76,16 +70,10 @@ class PAIMON_EXPORT StringUtils {
/// Replaces a String with another String inside a larger String, for the
first `max` values of
/// the search String.
///
- /// A `null` reference passed to this method is a no-op.
- ///
/// <pre>
- /// StringUtils::Replace(null, *, *, *) = null
/// StringUtils::Replace("", *, *, *) = ""
- /// StringUtils::Replace("any", null, *, *) = "any"
- /// StringUtils::Replace("any", *, null, *) = "any"
/// StringUtils::Replace("any", "", *, *) = "any"
/// StringUtils::Replace("any", *, *, 0) = "any"
- /// StringUtils::Replace("abaa", "a", null, -1) = "abaa"
/// StringUtils::Replace("abaa", "a", "", -1) = "b"
/// StringUtils::Replace("abaa", "a", "z", 0) = "abaa"
/// StringUtils::Replace("abaa", "a", "z", 1) = "zbaa"
@@ -93,11 +81,11 @@ class PAIMON_EXPORT StringUtils {
/// StringUtils::Replace("abaa", "a", "z", -1) = "zbzz"
/// </pre>
///
- /// @param text text to search and replace in, may be null
- /// @param search_string the String to search for, may be null
- /// @param replacement the String to replace it with, may be null
+ /// @param text text to search and replace in
+ /// @param search_string the String to search for
+ /// @param replacement the String to replace it with
/// @param max maximum number of values to replace, or `-1` if no maximum
- /// @return the text with any replacements processed, `null` if null
string input
+ /// @return the text with any replacements processed
static std::string Replace(const std::string& text, const std::string&
search_string,
const std::string& replacement, int32_t max);
@@ -115,6 +103,9 @@ class PAIMON_EXPORT StringUtils {
static std::string ToLowerCase(const std::string& str);
static std::string ToUpperCase(const std::string& str);
+ /// Compares two strings using ASCII case folding.
+ static bool EqualsIgnoreCase(const std::string& left, const std::string&
right);
+
template <typename T>
static std::string VectorToString(const std::vector<T>& vec) {
std::vector<std::string> strs;
diff --git a/src/paimon/common/utils/string_utils_test.cpp
b/src/paimon/common/utils/string_utils_test.cpp
index 11c3e000..a4f23078 100644
--- a/src/paimon/common/utils/string_utils_test.cpp
+++ b/src/paimon/common/utils/string_utils_test.cpp
@@ -73,6 +73,8 @@ void StringUtilsTest::CheckOverFlowAndUnderFlow(const
std::string& over_flow,
}
TEST_F(StringUtilsTest, TestReplaceAll) {
+ ASSERT_EQ("abc", StringUtils::Replace("abc", "", "x"));
+ ASSERT_EQ("", StringUtils::Replace("", "a", "b"));
{
std::string origin = "how is is you";
std::string expect = "how are are you";
@@ -118,6 +120,8 @@ TEST_F(StringUtilsTest, TestReplaceAll) {
}
TEST_F(StringUtilsTest, TestReplaceLast) {
+ ASSERT_EQ("abc", StringUtils::ReplaceLast("abc", "", "x"));
+ ASSERT_EQ("", StringUtils::ReplaceLast("", "a", "b"));
{
std::string origin = "a/b/c//";
std::string expect = "a/b/c/_";
@@ -140,6 +144,7 @@ TEST_F(StringUtilsTest, TestReplaceLast) {
}
TEST_F(StringUtilsTest, TestReplaceWithMaxCount) {
+ ASSERT_EQ("abc", StringUtils::Replace("abc", "a", "b", 0));
{
std::string origin = "how is is you";
std::string expect = "how are is you";
@@ -236,6 +241,13 @@ TEST_F(StringUtilsTest, TestToUpperCase) {
}
}
+TEST_F(StringUtilsTest, TestEqualsIgnoreCase) {
+ ASSERT_TRUE(StringUtils::EqualsIgnoreCase("", ""));
+ ASSERT_TRUE(StringUtils::EqualsIgnoreCase("AbC-123", "aBc-123"));
+ ASSERT_FALSE(StringUtils::EqualsIgnoreCase("abc", "abcd"));
+ ASSERT_FALSE(StringUtils::EqualsIgnoreCase("abc", "abx"));
+}
+
TEST_F(StringUtilsTest, TestStartsWith) {
{
std::string str = "abcde";
@@ -261,6 +273,26 @@ TEST_F(StringUtilsTest, TestStartsWith) {
std::string str = "";
ASSERT_TRUE(StringUtils::StartsWith(str, ""));
}
+ {
+ std::string str = "abc";
+ ASSERT_TRUE(StringUtils::StartsWith(str, "", /*start_pos=*/3));
+ ASSERT_FALSE(StringUtils::StartsWith(str, "", /*start_pos=*/4));
+ ASSERT_FALSE(StringUtils::StartsWith(str, "a", /*start_pos=*/4));
+ }
+}
+
+TEST_F(StringUtilsTest, TestTrim) {
+ std::string value = " \tabc\r\n";
+ StringUtils::Trim(&value);
+ ASSERT_EQ("abc", value);
+
+ value = "\t\r\n";
+ StringUtils::Trim(&value);
+ ASSERT_TRUE(value.empty());
+
+ value.clear();
+ StringUtils::Trim(&value);
+ ASSERT_TRUE(value.empty());
}
TEST_F(StringUtilsTest, TestEndsWith) {
{
diff --git a/src/paimon/core/core_options.cpp b/src/paimon/core/core_options.cpp
index 6320ec57..a5b32b09 100644
--- a/src/paimon/core/core_options.cpp
+++ b/src/paimon/core/core_options.cpp
@@ -19,6 +19,7 @@
#include "paimon/core/core_options.h"
#include <cstring>
+#include <initializer_list>
#include <limits>
#include <memory>
#include <optional>
@@ -51,14 +52,9 @@ class ConfigParser {
// Parse basic type configurations
template <typename T>
Status Parse(const std::string& key, T* value) const {
- auto iter = config_map_.find(key);
- if (iter != config_map_.end()) {
- auto result = StringUtils::StringToValue<T>(iter->second);
- if (result) {
- *value = result.value();
- return Status::OK();
- }
- return Status::Invalid(fmt::format("Invalid Config [{}: {}]", key,
iter->second));
+ PAIMON_ASSIGN_OR_RAISE(std::optional<T> parsed_value,
GetOptionalValue<T>(key));
+ if (parsed_value) {
+ *value = parsed_value.value();
}
return Status::OK(); // Return success even if the configuration does
not exist
}
@@ -66,14 +62,9 @@ class ConfigParser {
// Parse optional basic type configurations
template <typename T>
Status Parse(const std::string& key, std::optional<T>* value) const {
- auto iter = config_map_.find(key);
- if (iter != config_map_.end()) {
- auto result = StringUtils::StringToValue<T>(iter->second);
- if (result) {
- *value = result.value();
- return Status::OK();
- }
- return Status::Invalid(fmt::format("Invalid Config [{}: {}]", key,
iter->second));
+ PAIMON_ASSIGN_OR_RAISE(std::optional<T> parsed_value,
GetOptionalValue<T>(key));
+ if (parsed_value) {
+ *value = parsed_value.value();
}
return Status::OK(); // Return success even if the configuration does
not exist
}
@@ -82,23 +73,26 @@ class ConfigParser {
template <typename T>
Status ParseList(const std::string& key, const std::string& delimiter,
std::vector<T>* list,
bool need_trim = false) const {
- auto iter = config_map_.find(key);
- if (iter != config_map_.end()) {
- auto value_str_vec = StringUtils::Split(iter->second, delimiter,
/*ignore_empty=*/true);
- for (auto& value_str : value_str_vec) {
- if (need_trim) {
- StringUtils::Trim(&value_str);
- }
- if constexpr (std::is_same_v<T, std::string>) {
- list->emplace_back(value_str);
- } else {
- auto value = StringUtils::StringToValue<T>(value_str);
- if (!value) {
- return Status::Invalid(
- fmt::format("Invalid Config [{}: {}]", key,
iter->second));
- }
- list->emplace_back(value.value());
+ PAIMON_ASSIGN_OR_RAISE(std::optional<std::string> config_value,
+ GetOptionalValue<std::string>(key));
+ if (!config_value) {
+ return Status::OK();
+ }
+ auto value_str_vec =
+ StringUtils::Split(config_value.value(), delimiter,
/*ignore_empty=*/true);
+ for (auto& value_str : value_str_vec) {
+ if (need_trim) {
+ StringUtils::Trim(&value_str);
+ }
+ if constexpr (std::is_same_v<T, std::string>) {
+ list->emplace_back(value_str);
+ } else {
+ auto value = StringUtils::StringToValue<T>(value_str);
+ if (!value) {
+ return Status::Invalid(
+ fmt::format("Invalid Config [{}: {}]", key,
config_value.value()));
}
+ list->emplace_back(value.value());
}
}
return Status::OK(); // Return success even if the configuration does
not exist
@@ -109,9 +103,10 @@ class ConfigParser {
Status ParseMemorySize(const std::string& key, T* value) const {
static_assert(std::is_same_v<T, int64_t> || std::is_same_v<T,
std::optional<int64_t>>,
"ParseMemorySize only supports int64_t and
std::optional<int64_t>");
- auto iter = config_map_.find(key);
- if (iter != config_map_.end()) {
- PAIMON_ASSIGN_OR_RAISE(*value,
MemorySize::ParseBytes(iter->second));
+ PAIMON_ASSIGN_OR_RAISE(std::optional<std::string> config_value,
+ GetOptionalValue<std::string>(key));
+ if (config_value) {
+ PAIMON_ASSIGN_OR_RAISE(*value,
MemorySize::ParseBytes(config_value.value()));
}
return Status::OK();
}
@@ -121,9 +116,10 @@ class ConfigParser {
Status ParseTimeDuration(const std::string& key, T* value) const {
static_assert(std::is_same_v<T, int64_t> || std::is_same_v<T,
std::optional<int64_t>>,
"ParseTimeDuration only supports int64_t and
std::optional<int64_t>");
- auto iter = config_map_.find(key);
- if (iter != config_map_.end()) {
- PAIMON_ASSIGN_OR_RAISE(*value, TimeDuration::Parse(iter->second));
+ PAIMON_ASSIGN_OR_RAISE(std::optional<std::string> config_value,
+ GetOptionalValue<std::string>(key));
+ if (config_value) {
+ PAIMON_ASSIGN_OR_RAISE(*value,
TimeDuration::Parse(config_value.value()));
}
return Status::OK();
}
@@ -132,14 +128,10 @@ class ConfigParser {
template <typename Factory, typename ObjectType>
Status ParseObject(const std::string& key, const std::string&
default_identifier,
std::shared_ptr<ObjectType>* value) const {
- auto iter = config_map_.find(key);
- if (iter != config_map_.end()) {
- std::string normalized_value =
StringUtils::ToLowerCase(iter->second);
- PAIMON_ASSIGN_OR_RAISE(*value, Factory::Get(normalized_value,
config_map_));
- } else {
- PAIMON_ASSIGN_OR_RAISE(
- *value,
Factory::Get(StringUtils::ToLowerCase(default_identifier), config_map_));
- }
+ PAIMON_ASSIGN_OR_RAISE(std::string identifier,
OptionsUtils::GetValueFromMap<std::string>(
+ config_map_, key,
default_identifier));
+ PAIMON_ASSIGN_OR_RAISE(*value,
+
Factory::Get(StringUtils::ToLowerCase(identifier), config_map_));
return Status::OK();
}
@@ -152,11 +144,10 @@ class ConfigParser {
*value = specified_file_system;
return Status::OK();
}
- std::string default_fs_identifier = "local";
- auto iter = config_map_.find(Options::FILE_SYSTEM);
- if (iter != config_map_.end()) {
- default_fs_identifier = StringUtils::ToLowerCase(iter->second);
- }
+ PAIMON_ASSIGN_OR_RAISE(
+ std::string default_fs_identifier,
+ OptionsUtils::GetValueFromMap<std::string>(config_map_,
Options::FILE_SYSTEM, "local"));
+ default_fs_identifier =
StringUtils::ToLowerCase(default_fs_identifier);
*value =
std::make_shared<ResolvingFileSystem>(fs_scheme_to_identifier_map,
default_fs_identifier,
config_map_);
return Status::OK();
@@ -164,151 +155,81 @@ class ConfigParser {
// Parse SortOrder
Status ParseSortOrder(SortOrder* sort_order) const {
- auto iter = config_map_.find(Options::SEQUENCE_FIELD_SORT_ORDER);
- if (iter != config_map_.end()) {
- std::string str = StringUtils::ToLowerCase(iter->second);
- if (str == "ascending") {
- *sort_order = SortOrder::ASCENDING;
- } else if (str == "descending") {
- *sort_order = SortOrder::DESCENDING;
- } else {
- return Status::Invalid(fmt::format("invalid sort order: {}",
str));
- }
- }
- return Status::OK();
+ return ParseEnum(
+ Options::SEQUENCE_FIELD_SORT_ORDER,
+ {{"ascending", SortOrder::ASCENDING}, {"descending",
SortOrder::DESCENDING}},
+ "sort order", sort_order);
}
// Parse LookupCompactMode
Status ParseLookupCompactMode(LookupCompactMode* mode) const {
- auto iter = config_map_.find(Options::LOOKUP_COMPACT);
- if (iter != config_map_.end()) {
- std::string str = StringUtils::ToLowerCase(iter->second);
- if (str == "radical") {
- *mode = LookupCompactMode::RADICAL;
- } else if (str == "gentle") {
- *mode = LookupCompactMode::GENTLE;
- } else {
- return Status::Invalid(fmt::format("invalid lookup mode: {}",
str));
- }
- }
- return Status::OK();
+ return ParseEnum(
+ Options::LOOKUP_COMPACT,
+ {{"radical", LookupCompactMode::RADICAL}, {"gentle",
LookupCompactMode::GENTLE}},
+ "lookup mode", mode);
}
// Parse SortEngine
Status ParseSortEngine(SortEngine* sort_engine) const {
- auto iter = config_map_.find(Options::SORT_ENGINE);
- if (iter != config_map_.end()) {
- std::string str = StringUtils::ToLowerCase(iter->second);
- if (str == "min-heap") {
- *sort_engine = SortEngine::MIN_HEAP;
- } else if (str == "loser-tree") {
- *sort_engine = SortEngine::LOSER_TREE;
- } else {
- return Status::Invalid(fmt::format("invalid sort engine: {}",
str));
- }
- }
- return Status::OK();
+ return ParseEnum(
+ Options::SORT_ENGINE,
+ {{"min-heap", SortEngine::MIN_HEAP}, {"loser-tree",
SortEngine::LOSER_TREE}},
+ "sort engine", sort_engine);
}
// Parse MergeEngine
Status ParseMergeEngine(MergeEngine* merge_engine) const {
- auto iter = config_map_.find(Options::MERGE_ENGINE);
- if (iter != config_map_.end()) {
- std::string str = StringUtils::ToLowerCase(iter->second);
- if (str == "deduplicate") {
- *merge_engine = MergeEngine::DEDUPLICATE;
- } else if (str == "partial-update") {
- *merge_engine = MergeEngine::PARTIAL_UPDATE;
- } else if (str == "aggregation") {
- *merge_engine = MergeEngine::AGGREGATE;
- } else if (str == "first-row") {
- *merge_engine = MergeEngine::FIRST_ROW;
- } else {
- return Status::Invalid(fmt::format("invalid merge engine: {}",
str));
- }
- }
- return Status::OK();
+ return ParseEnum(Options::MERGE_ENGINE,
+ {{"deduplicate", MergeEngine::DEDUPLICATE},
+ {"partial-update", MergeEngine::PARTIAL_UPDATE},
+ {"aggregation", MergeEngine::AGGREGATE},
+ {"first-row", MergeEngine::FIRST_ROW}},
+ "merge engine", merge_engine);
}
// Parse VariantShreddingInferenceMode
Status ParseVariantShreddingInferenceMode(VariantShreddingInferenceMode*
inference_mode) const {
- auto iter =
config_map_.find(Options::VARIANT_SHREDDING_INFERENCE_MODE);
- if (iter != config_map_.end()) {
- std::string str = StringUtils::ToLowerCase(iter->second);
- if (str == "per-file") {
- *inference_mode = VariantShreddingInferenceMode::PER_FILE;
- } else if (str == "adaptive") {
- *inference_mode = VariantShreddingInferenceMode::ADAPTIVE;
- } else {
- return Status::Invalid(
- fmt::format("invalid variant shredding inference mode:
{}", str));
- }
- }
- return Status::OK();
+ return ParseEnum(Options::VARIANT_SHREDDING_INFERENCE_MODE,
+ {{"per-file",
VariantShreddingInferenceMode::PER_FILE},
+ {"adaptive",
VariantShreddingInferenceMode::ADAPTIVE}},
+ "variant shredding inference mode", inference_mode);
}
// Parse ChangelogProducer
Status ParseChangelogProducer(ChangelogProducer* changelog_producer) const
{
- auto iter = config_map_.find(Options::CHANGELOG_PRODUCER);
- if (iter != config_map_.end()) {
- std::string str = StringUtils::ToLowerCase(iter->second);
- if (str == "none") {
- *changelog_producer = ChangelogProducer::NONE;
- } else if (str == "input") {
- *changelog_producer = ChangelogProducer::INPUT;
- } else if (str == "full-compaction") {
- *changelog_producer = ChangelogProducer::FULL_COMPACTION;
- } else if (str == "lookup") {
- *changelog_producer = ChangelogProducer::LOOKUP;
- } else {
- return Status::Invalid(fmt::format("invalid changelog
producer: {}", str));
- }
- }
- return Status::OK();
+ return ParseEnum(Options::CHANGELOG_PRODUCER,
+ {{"none", ChangelogProducer::NONE},
+ {"input", ChangelogProducer::INPUT},
+ {"full-compaction",
ChangelogProducer::FULL_COMPACTION},
+ {"lookup", ChangelogProducer::LOOKUP}},
+ "changelog producer", changelog_producer);
}
// Parse ExternalPathStrategy
Status ParseExternalPathStrategy(ExternalPathStrategy*
external_path_strategy) const {
- auto iter =
config_map_.find(Options::DATA_FILE_EXTERNAL_PATHS_STRATEGY);
- if (iter != config_map_.end()) {
- std::string str = StringUtils::ToLowerCase(iter->second);
- if (str == "none") {
- *external_path_strategy = ExternalPathStrategy::NONE;
- } else if (str == "specific-fs") {
- *external_path_strategy = ExternalPathStrategy::SPECIFIC_FS;
- } else if (str == "round-robin") {
- *external_path_strategy = ExternalPathStrategy::ROUND_ROBIN;
- } else {
- return Status::Invalid(fmt::format("invalid external path
strategy: {}", str));
- }
- }
- return Status::OK();
+ return ParseEnum(Options::DATA_FILE_EXTERNAL_PATHS_STRATEGY,
+ {{"none", ExternalPathStrategy::NONE},
+ {"specific-fs", ExternalPathStrategy::SPECIFIC_FS},
+ {"round-robin", ExternalPathStrategy::ROUND_ROBIN}},
+ "external path strategy", external_path_strategy);
}
// Parse BucketFunctionType
Status ParseBucketFunctionType(BucketFunctionType* bucket_function_type)
const {
- auto iter = config_map_.find(Options::BUCKET_FUNCTION_TYPE);
- if (iter != config_map_.end()) {
- std::string str = StringUtils::ToLowerCase(iter->second);
- if (str == "default") {
- *bucket_function_type = BucketFunctionType::DEFAULT;
- } else if (str == "mod") {
- *bucket_function_type = BucketFunctionType::MOD;
- } else if (str == "hive") {
- *bucket_function_type = BucketFunctionType::HIVE;
- } else {
- return Status::Invalid(fmt::format("invalid bucket function
type: {}", str));
- }
- }
- return Status::OK();
+ return ParseEnum(Options::BUCKET_FUNCTION_TYPE,
+ {{"default", BucketFunctionType::DEFAULT},
+ {"mod", BucketFunctionType::MOD},
+ {"hive", BucketFunctionType::HIVE}},
+ "bucket function type", bucket_function_type);
}
// Parse StartupMode
Status ParseStartupMode(StartupMode* startup_mode) const {
- auto iter = config_map_.find(Options::SCAN_MODE);
- if (iter != config_map_.end()) {
- std::string str = StringUtils::ToLowerCase(iter->second);
- PAIMON_ASSIGN_OR_RAISE(*startup_mode,
StartupMode::FromString(str));
+ PAIMON_ASSIGN_OR_RAISE(std::optional<std::string> value,
+
GetOptionalValue<std::string>(Options::SCAN_MODE));
+ if (value) {
+ PAIMON_ASSIGN_OR_RAISE(
+ *startup_mode,
StartupMode::FromString(StringUtils::ToLowerCase(value.value())));
}
return Status::OK();
}
@@ -372,7 +293,37 @@ class ConfigParser {
}
private:
- const std::map<std::string, std::string> config_map_;
+ template <typename T>
+ Status ParseEnum(const std::string& key,
+ std::initializer_list<std::pair<const char*, T>>
candidates,
+ const std::string& error_name, T* value) const {
+ PAIMON_ASSIGN_OR_RAISE(std::optional<std::string> config_value,
+ GetOptionalValue<std::string>(key));
+ if (!config_value) {
+ return Status::OK();
+ }
+ std::string normalized_value =
StringUtils::ToLowerCase(config_value.value());
+ for (const auto& [candidate, candidate_value] : candidates) {
+ if (normalized_value == candidate) {
+ *value = candidate_value;
+ return Status::OK();
+ }
+ }
+ return Status::Invalid(fmt::format("invalid {}: {}", error_name,
normalized_value));
+ }
+
+ template <typename T>
+ Result<std::optional<T>> GetOptionalValue(const std::string& key) const {
+ Result<std::optional<T>> result =
+ OptionsUtils::GetOptionalValueFromMap<T>(config_map_, key);
+ if (!result.ok()) {
+ return Status::Invalid(
+ fmt::format("Invalid Config [{}: {}]", key,
config_map_.at(key)));
+ }
+ return result.value();
+ }
+
+ const std::map<std::string, std::string>& config_map_;
};
// Impl is a private implementation of CoreOptions,
diff --git
a/src/paimon/core/mergetree/compact/lookup_merge_tree_compact_rewriter.cpp
b/src/paimon/core/mergetree/compact/lookup_merge_tree_compact_rewriter.cpp
index 994b0e3f..071456e4 100644
--- a/src/paimon/core/mergetree/compact/lookup_merge_tree_compact_rewriter.cpp
+++ b/src/paimon/core/mergetree/compact/lookup_merge_tree_compact_rewriter.cpp
@@ -78,14 +78,9 @@ LookupMergeTreeCompactRewriter<T>::Create(
.WithMemoryPool(pool);
PAIMON_ASSIGN_OR_RAISE(std::shared_ptr<ReadContext> read_context,
read_context_builder.Finish());
- // TODO(xinyu.lxy): temporarily disabled pre-buffer for parquet, which may
cause high memory
- // usage during compaction. Will fix via parquet format refactor.
- auto new_options = options.ToMap();
- if (new_options.find("parquet.read.enable-pre-buffer") ==
new_options.end()) {
- new_options["parquet.read.enable-pre-buffer"] = "false";
- }
- PAIMON_ASSIGN_OR_RAISE(std::shared_ptr<InternalReadContext>
internal_context,
- InternalReadContext::Create(read_context,
table_schema, new_options));
+ PAIMON_ASSIGN_OR_RAISE(
+ std::shared_ptr<InternalReadContext> internal_context,
+ InternalReadContext::Create(read_context, table_schema,
options.ToMap()));
PAIMON_ASSIGN_OR_RAISE(
std::shared_ptr<FileStorePathFactory> path_factory,
path_factory_cache->GetOrCreatePathFactory(options.GetFileFormat()->Identifier()));
diff --git a/src/paimon/core/mergetree/compact/merge_tree_compact_rewriter.cpp
b/src/paimon/core/mergetree/compact/merge_tree_compact_rewriter.cpp
index 1b64be2d..fd7c7cbd 100644
--- a/src/paimon/core/mergetree/compact/merge_tree_compact_rewriter.cpp
+++ b/src/paimon/core/mergetree/compact/merge_tree_compact_rewriter.cpp
@@ -84,14 +84,9 @@ Result<std::unique_ptr<MergeTreeCompactRewriter>>
MergeTreeCompactRewriter::Crea
.WithMemoryPool(pool);
PAIMON_ASSIGN_OR_RAISE(std::shared_ptr<ReadContext> read_context,
read_context_builder.Finish());
- // TODO(xinyu.lxy): temporarily disabled pre-buffer for parquet, which may
cause high memory
- // usage during compaction. Will fix via parquet format refactor.
- auto new_options = options.ToMap();
- if (new_options.find("parquet.read.enable-pre-buffer") ==
new_options.end()) {
- new_options["parquet.read.enable-pre-buffer"] = "false";
- }
- PAIMON_ASSIGN_OR_RAISE(std::shared_ptr<InternalReadContext>
internal_context,
- InternalReadContext::Create(read_context,
table_schema, new_options));
+ PAIMON_ASSIGN_OR_RAISE(
+ std::shared_ptr<InternalReadContext> internal_context,
+ InternalReadContext::Create(read_context, table_schema,
options.ToMap()));
PAIMON_ASSIGN_OR_RAISE(
std::shared_ptr<FileStorePathFactory> path_factory,
path_factory_cache->GetOrCreatePathFactory(options.GetFileFormat()->Identifier()));
diff --git a/src/paimon/core/mergetree/lookup_levels.cpp
b/src/paimon/core/mergetree/lookup_levels.cpp
index a931fea1..512f2872 100644
--- a/src/paimon/core/mergetree/lookup_levels.cpp
+++ b/src/paimon/core/mergetree/lookup_levels.cpp
@@ -66,14 +66,9 @@ Result<std::unique_ptr<LookupLevels<T>>>
LookupLevels<T>::Create(
.WithMemoryPool(pool);
PAIMON_ASSIGN_OR_RAISE(std::shared_ptr<ReadContext> read_context,
read_context_builder.Finish());
- // TODO(xinyu.lxy): temporarily disabled pre-buffer for parquet, which may
cause high memory
- // usage during compaction. Will fix via parquet format refactor.
- auto new_options = options.ToMap();
- if (new_options.find("parquet.read.enable-pre-buffer") ==
new_options.end()) {
- new_options["parquet.read.enable-pre-buffer"] = "false";
- }
- PAIMON_ASSIGN_OR_RAISE(std::shared_ptr<InternalReadContext>
internal_read_context,
- InternalReadContext::Create(read_context,
table_schema, new_options));
+ PAIMON_ASSIGN_OR_RAISE(
+ std::shared_ptr<InternalReadContext> internal_read_context,
+ InternalReadContext::Create(read_context, table_schema,
options.ToMap()));
auto split_read = std::make_unique<RawFileSplitRead>(path_factory,
internal_read_context, pool,
CreateDefaultExecutor());
diff --git a/src/paimon/core/operation/append_only_file_store_write.cpp
b/src/paimon/core/operation/append_only_file_store_write.cpp
index 5d6c2c93..f660093a 100644
--- a/src/paimon/core/operation/append_only_file_store_write.cpp
+++ b/src/paimon/core/operation/append_only_file_store_write.cpp
@@ -290,14 +290,8 @@ Result<std::unique_ptr<BatchReader>>
AppendOnlyFileStoreWrite::CreateFilesReader
.WithMemoryPool(pool_);
PAIMON_ASSIGN_OR_RAISE(std::shared_ptr<ReadContext> read_context,
context_builder.Finish());
std::map<std::string, std::string> options = options_.ToMap();
- // TODO(xinyu.lxy): temporarily disabled pre-buffer for parquet, which may
cause high
- // memory usage during compaction. Will fix via parquet format refactor.
- auto new_options = options;
- if (new_options.find("parquet.read.enable-pre-buffer") ==
new_options.end()) {
- new_options["parquet.read.enable-pre-buffer"] = "false";
- }
PAIMON_ASSIGN_OR_RAISE(std::shared_ptr<InternalReadContext>
internal_read_context,
- InternalReadContext::Create(read_context,
table_schema_, new_options));
+ InternalReadContext::Create(read_context,
table_schema_, options));
auto read = std::make_unique<RawFileSplitRead>(file_store_path_factory_,
internal_read_context,
pool_, compact_executor_);
diff --git a/src/paimon/core/operation/commit/sequence_snapshot_properties.cpp
b/src/paimon/core/operation/commit/sequence_snapshot_properties.cpp
index 51729b99..512869f0 100644
--- a/src/paimon/core/operation/commit/sequence_snapshot_properties.cpp
+++ b/src/paimon/core/operation/commit/sequence_snapshot_properties.cpp
@@ -24,6 +24,7 @@
#include <algorithm>
#include <limits>
+#include "paimon/common/utils/string_utils.h"
#include "paimon/core/manifest/file_kind.h"
namespace paimon {
@@ -40,19 +41,12 @@ Result<std::optional<int64_t>>
SequenceSnapshotProperties::MaxSequenceNumber(
return std::optional<int64_t>();
}
- try {
- size_t parsed = 0;
- int64_t value = std::stoll(iter->second, &parsed);
- if (parsed != iter->second.size()) {
- return Status::Invalid(
- fmt::format("Invalid {} value '{}': trailing characters are
not allowed",
- kMaxSequenceNumberKey, iter->second));
- }
- return std::optional<int64_t>(value);
- } catch (const std::exception& e) {
- return Status::Invalid(fmt::format("Invalid {} value '{}': {}",
kMaxSequenceNumberKey,
- iter->second, e.what()));
+ std::optional<int64_t> value =
StringUtils::StringToValue<int64_t>(iter->second);
+ if (!value) {
+ return Status::Invalid(
+ fmt::format("Invalid {} value '{}'", kMaxSequenceNumberKey,
iter->second));
}
+ return value;
}
std::optional<int64_t> SequenceSnapshotProperties::MaxSequenceNumberFromFiles(
diff --git
a/src/paimon/core/operation/commit/sequence_snapshot_properties_test.cpp
b/src/paimon/core/operation/commit/sequence_snapshot_properties_test.cpp
index 5975eb1e..ca962e0e 100644
--- a/src/paimon/core/operation/commit/sequence_snapshot_properties_test.cpp
+++ b/src/paimon/core/operation/commit/sequence_snapshot_properties_test.cpp
@@ -115,7 +115,7 @@ TEST_F(SequenceSnapshotPropertiesTest,
MaxSequenceNumberTrailingCharacters) {
std::map<std::string, std::string> properties{
{SequenceSnapshotProperties::kMaxSequenceNumberKey, "123abc"}};
ASSERT_NOK_WITH_MSG(SequenceSnapshotProperties::MaxSequenceNumber(MakeSnapshot(properties)),
- "trailing characters are not allowed");
+ "Invalid sequence.generation.max-sequence-number value
'123abc'");
}
TEST_F(SequenceSnapshotPropertiesTest, MaxSequenceNumberNotANumber) {
diff --git a/src/paimon/core/postpone/postpone_bucket_writer.cpp
b/src/paimon/core/postpone/postpone_bucket_writer.cpp
index 47fd3bcb..45bcfd36 100644
--- a/src/paimon/core/postpone/postpone_bucket_writer.cpp
+++ b/src/paimon/core/postpone/postpone_bucket_writer.cpp
@@ -34,7 +34,6 @@
#include "paimon/common/data/shredding/shredding_write_plan_factories.h"
#include "paimon/common/metrics/metrics_impl.h"
#include "paimon/common/table/special_fields.h"
-#include "paimon/common/types/data_field.h"
#include "paimon/common/types/row_kind.h"
#include "paimon/common/utils/arrow/mem_utils.h"
#include "paimon/common/utils/arrow/status_utils.h"
@@ -55,27 +54,12 @@ namespace paimon {
class InternalRow;
class MemoryPool;
-namespace {
-
-std::shared_ptr<arrow::Schema> BuildPostponeBucketWriteSchema(
- const std::shared_ptr<arrow::Schema>& value_schema) {
- arrow::FieldVector target_fields;
- target_fields.push_back(
-
DataField::ConvertDataFieldToArrowField(SpecialFields::SequenceNumber()));
-
target_fields.push_back(DataField::ConvertDataFieldToArrowField(SpecialFields::ValueKind()));
- target_fields.insert(target_fields.end(), value_schema->fields().begin(),
- value_schema->fields().end());
- return arrow::schema(target_fields);
-}
-
-} // namespace
-
Result<std::unique_ptr<PostponeBucketWriter>> PostponeBucketWriter::Create(
const std::vector<std::string>& trimmed_primary_keys,
const std::shared_ptr<DataFilePathFactory>& path_factory, int64_t
schema_id,
const std::shared_ptr<arrow::Schema>& value_schema, const CoreOptions&
options,
const std::shared_ptr<MemoryPool>& pool) {
- auto write_schema = BuildPostponeBucketWriteSchema(value_schema);
+ auto write_schema =
SpecialFields::CompleteSequenceAndValueKindField(value_schema);
return std::unique_ptr<PostponeBucketWriter>(new PostponeBucketWriter(
trimmed_primary_keys, path_factory, schema_id, value_schema,
write_schema, options, pool));
}
diff --git a/src/paimon/core/schema/schema_validation.cpp
b/src/paimon/core/schema/schema_validation.cpp
index 7342826d..4c7dd2ce 100644
--- a/src/paimon/core/schema/schema_validation.cpp
+++ b/src/paimon/core/schema/schema_validation.cpp
@@ -100,7 +100,7 @@ Status ValidateSharedShreddingFileFormat(const std::string&
option_key,
}
Status ValidateVectorFileFormat(const std::string& option_key, const
std::string& file_format) {
- if (StringUtils::ToLowerCase(file_format) != "parquet") {
+ if (!StringUtils::EqualsIgnoreCase(file_format, "parquet")) {
return Status::Invalid(
fmt::format("VECTOR currently only supports parquet data files,
but {} is {}.",
option_key, file_format));
diff --git a/src/paimon/core/schema/schema_validation_test.cpp
b/src/paimon/core/schema/schema_validation_test.cpp
index 47603497..57885626 100644
--- a/src/paimon/core/schema/schema_validation_test.cpp
+++ b/src/paimon/core/schema/schema_validation_test.cpp
@@ -56,6 +56,12 @@ TEST(SchemaValidationTest, TestVectorType) {
/*primary_keys=*/{},
parquet_options));
ASSERT_OK(SchemaValidation::ValidateTableSchema(*table_schema));
+ parquet_options[Options::FILE_FORMAT] = "PARQUET";
+ ASSERT_OK_AND_ASSIGN(table_schema,
+ TableSchema::Create(/*schema_id=*/0, schema,
/*partition_keys=*/{},
+ /*primary_keys=*/{},
parquet_options));
+ ASSERT_OK(SchemaValidation::ValidateTableSchema(*table_schema));
+
std::map<std::string, std::string> orc_options = {{Options::BUCKET, "-1"},
{Options::FILE_FORMAT,
"orc"}};
ASSERT_OK_AND_ASSIGN(table_schema,
diff --git a/src/paimon/core/table/system/global_system_tables.cpp
b/src/paimon/core/table/system/global_system_tables.cpp
index dd1b4e60..6523588e 100644
--- a/src/paimon/core/table/system/global_system_tables.cpp
+++ b/src/paimon/core/table/system/global_system_tables.cpp
@@ -108,11 +108,12 @@ VariantType OptionalStringValue(const
std::map<std::string, std::string>& option
Result<VariantType> OptionalLongValue(const std::map<std::string,
std::string>& options,
const std::string& key) {
- if (options.find(key) == options.end()) {
+ PAIMON_ASSIGN_OR_RAISE(std::optional<int64_t> value,
+
OptionsUtils::GetOptionalValueFromMap<int64_t>(options, key));
+ if (!value) {
return VariantType(NullType());
}
- PAIMON_ASSIGN_OR_RAISE(int64_t value,
OptionsUtils::GetValueFromMap<int64_t>(options, key));
- return VariantType(value);
+ return VariantType(value.value());
}
Result<bool> IsEnabled(const GlobalSystemTableRegistryEntry& entry,
diff --git a/src/paimon/format/orc/orc_format_writer.cpp
b/src/paimon/format/orc/orc_format_writer.cpp
index 1a394ca9..fc2316b2 100644
--- a/src/paimon/format/orc/orc_format_writer.cpp
+++ b/src/paimon/format/orc/orc_format_writer.cpp
@@ -40,7 +40,6 @@
#include "orc/Writer.hh"
#include "paimon/common/data/variant/variant_type_utils.h"
#include "paimon/common/metrics/metrics_impl.h"
-#include "paimon/common/options/memory_size.h"
#include "paimon/common/utils/arrow/status_utils.h"
#include "paimon/common/utils/options_utils.h"
#include "paimon/common/utils/string_utils.h"
@@ -236,18 +235,6 @@ Status OrcFormatWriter::AddMetadata(const
std::map<std::string, std::string>& me
return Status::OK();
}
-namespace {
-
-Result<uint64_t> GetMemorySizeOption(const std::map<std::string, std::string>&
options,
- const std::string& key, uint64_t
default_value) {
- PAIMON_ASSIGN_OR_RAISE(std::string value,
OptionsUtils::GetValueFromMap<std::string>(
- options, key,
std::to_string(default_value)));
- PAIMON_ASSIGN_OR_RAISE(int64_t bytes, MemorySize::ParseBytes(value));
- return static_cast<uint64_t>(bytes);
-}
-
-} // namespace
-
Result<::orc::WriterOptions> OrcFormatWriter::PrepareWriterOptions(
const std::map<std::string, std::string>& options, const std::string&
file_compression,
const std::shared_ptr<arrow::DataType>& data_type) {
@@ -261,15 +248,16 @@ Result<::orc::WriterOptions>
OrcFormatWriter::PrepareWriterOptions(
}
}
::orc::WriterOptions writer_options;
- PAIMON_ASSIGN_OR_RAISE(uint64_t stripe_size,
- GetMemorySizeOption(options, ORC_STRIPE_SIZE,
DEFAULT_STRIPE_SIZE));
+ PAIMON_ASSIGN_OR_RAISE(
+ uint64_t stripe_size,
+ OptionsUtils::GetValueFromMap<uint64_t>(options, ORC_STRIPE_SIZE,
DEFAULT_STRIPE_SIZE));
writer_options.setStripeSize(stripe_size);
PAIMON_ASSIGN_OR_RAISE(::orc::CompressionKind compression,
ToOrcCompressionKind(StringUtils::ToLowerCase(file_compression)));
writer_options.setCompression(compression);
- PAIMON_ASSIGN_OR_RAISE(
- uint64_t compression_block_size,
- GetMemorySizeOption(options, ORC_COMPRESSION_BLOCK_SIZE,
DEFAULT_COMPRESSION_BLOCK_SIZE));
+ PAIMON_ASSIGN_OR_RAISE(uint64_t compression_block_size,
OptionsUtils::GetValueFromMap<uint64_t>(
+ options,
ORC_COMPRESSION_BLOCK_SIZE,
+
DEFAULT_COMPRESSION_BLOCK_SIZE));
writer_options.setCompressionBlockSize(compression_block_size);
PAIMON_ASSIGN_OR_RAISE(
double dictionary_key_threshold,
diff --git a/src/paimon/format/parquet/parquet_format_defs.h
b/src/paimon/format/parquet/parquet_format_defs.h
index 433103e5..8b205a09 100644
--- a/src/paimon/format/parquet/parquet_format_defs.h
+++ b/src/paimon/format/parquet/parquet_format_defs.h
@@ -99,7 +99,7 @@ static inline const char
PARQUET_READ_PREDICATE_NODE_COUNT_LIMIT[] =
static inline const char PARQUET_READ_ENABLE_PAGE_INDEX_FILTER[] =
"parquet.read.enable-page-index-filter";
-// Default is true. Compaction will set to false to reduce memory consumption.
+// Default is true.
static inline const char PARQUET_READ_ENABLE_PRE_BUFFER[] =
"parquet.read.enable-pre-buffer";
static constexpr uint32_t DEFAULT_PARQUET_READ_CACHE_OPTION_PREFETCH_LIMIT = 0;
diff --git a/src/paimon/fs/local/local_file.cpp
b/src/paimon/fs/local/local_file.cpp
index 64530667..a3f960af 100644
--- a/src/paimon/fs/local/local_file.cpp
+++ b/src/paimon/fs/local/local_file.cpp
@@ -49,7 +49,7 @@ Result<std::unique_ptr<LocalFile>> LocalFile::Create(const
std::string& path_str
// local file system does not support path_string with scheme, e.g.,
"file:/tmp" will be
// rewritten to "/tmp"
PAIMON_ASSIGN_OR_RAISE(Path path, PathUtil::ToPath(path_string));
- if (!path.scheme.empty() && StringUtils::ToLowerCase(path.scheme) !=
"file") {
+ if (!path.scheme.empty() && !StringUtils::EqualsIgnoreCase(path.scheme,
"file")) {
return Status::Invalid(fmt::format("invalid scheme {} for local file
system", path.scheme));
}
if (path.path.empty() || path.path[0] != '/') {
diff --git a/src/paimon/fs/local/local_file_test.cpp
b/src/paimon/fs/local/local_file_test.cpp
index f22095c9..25b9f6db 100644
--- a/src/paimon/fs/local/local_file_test.cpp
+++ b/src/paimon/fs/local/local_file_test.cpp
@@ -27,6 +27,11 @@
namespace paimon::test {
+TEST(LocalFileTest, TestSchemeCaseInsensitive) {
+ ASSERT_OK(LocalFile::Create("FILE:/tmp"));
+ ASSERT_NOK(LocalFile::Create("s3:/tmp"));
+}
+
TEST(LocalFileTest, TestReadWriteEmptyContent) {
auto test_root_dir = UniqueTestDirectory::Create();
ASSERT_TRUE(test_root_dir);
diff --git a/src/paimon/fs/s3/s3_file_system.cpp
b/src/paimon/fs/s3/s3_file_system.cpp
index 49668b70..e31cc9ca 100644
--- a/src/paimon/fs/s3/s3_file_system.cpp
+++ b/src/paimon/fs/s3/s3_file_system.cpp
@@ -580,22 +580,22 @@ bool IsIpAddressAuthority(const std::string& authority) {
}
const char* AwsDnsSuffixForRegion(const std::string& region) {
- if (region.rfind("cn-", 0) == 0) {
+ if (StringUtils::StartsWith(region, "cn-")) {
return "amazonaws.com.cn";
}
- if (region.rfind("eusc-de-", 0) == 0) {
+ if (StringUtils::StartsWith(region, "eusc-de-")) {
return "amazonaws.eu";
}
- if (region.rfind("us-iso-", 0) == 0) {
+ if (StringUtils::StartsWith(region, "us-iso-")) {
return "c2s.ic.gov";
}
- if (region.rfind("us-isob-", 0) == 0) {
+ if (StringUtils::StartsWith(region, "us-isob-")) {
return "sc2s.sgov.gov";
}
- if (region.rfind("eu-isoe-", 0) == 0) {
+ if (StringUtils::StartsWith(region, "eu-isoe-")) {
return "cloud.adc-e.uk";
}
- if (region.rfind("us-isof-", 0) == 0) {
+ if (StringUtils::StartsWith(region, "us-isof-")) {
return "csp.hci.ic.gov";
}
return "amazonaws.com";
diff --git a/src/paimon/global_index/lucene/jieba_analyzer.cpp
b/src/paimon/global_index/lucene/jieba_analyzer.cpp
index 39cecec2..e0a71a81 100644
--- a/src/paimon/global_index/lucene/jieba_analyzer.cpp
+++ b/src/paimon/global_index/lucene/jieba_analyzer.cpp
@@ -17,6 +17,8 @@
*/
#include "paimon/global_index/lucene/jieba_analyzer.h"
+#include <cctype>
+
#include "paimon/common/utils/string_utils.h"
#include "paimon/global_index/lucene/lucene_utils.h"
@@ -94,9 +96,7 @@ void JiebaTokenizer::NormalizeCase(std::string* term) {
}
}
if (is_alphanumeric && !term->empty()) {
- std::transform(term->begin(), term->end(), term->begin(), [](char ch) {
- return static_cast<char>(std::tolower(static_cast<unsigned
char>(ch)));
- });
+ *term = StringUtils::ToLowerCase(*term);
}
}
diff --git a/src/paimon/rest/dlf_auth.cpp b/src/paimon/rest/dlf_auth.cpp
index 4c592f31..6a490627 100644
--- a/src/paimon/rest/dlf_auth.cpp
+++ b/src/paimon/rest/dlf_auth.cpp
@@ -22,7 +22,6 @@
#include <openssl/evp.h>
#include <array>
-#include <cctype>
#include <climits>
#include <ctime>
#include <fstream>
@@ -74,28 +73,10 @@ constexpr const char kAcsSignatureVersionHeader[] =
"x-acs-signature-version";
constexpr const char kAcsVersionHeader[] = "x-acs-version";
constexpr const char kAcsSecurityTokenHeader[] = "x-acs-security-token";
-void TrimWhitespace(std::string* value) {
- size_t begin = 0;
- while (begin < value->size() && std::isspace(static_cast<unsigned
char>((*value)[begin]))) {
- ++begin;
- }
- size_t end = value->size();
- while (end > begin && std::isspace(static_cast<unsigned char>((*value)[end
- 1]))) {
- --end;
- }
- *value = value->substr(begin, end - begin);
-}
-
Result<std::string> RequiredNonEmptyOption(const std::map<std::string,
std::string>& options,
const std::string& key) {
- Result<std::string> value =
OptionsUtils::GetValueFromMap<std::string>(options, key);
+ Result<std::string> value = OptionsUtils::GetNonEmptyValueFromMap(options,
key);
if (!value.ok()) {
- if (!value.status().IsNotExist()) {
- return value.status();
- }
- return Status::Invalid(fmt::format("option '{}' must be configured for
DLF auth", key));
- }
- if (value.value().empty()) {
return Status::Invalid(fmt::format("option '{}' must be configured for
DLF auth", key));
}
return value.value();
@@ -267,7 +248,7 @@ Result<std::string> Md5Base64(const std::string& value) {
std::string Trimmed(const std::string& value) {
std::string trimmed = value;
- TrimWhitespace(&trimmed);
+ StringUtils::Trim(&trimmed);
return trimmed;
}
@@ -510,7 +491,7 @@ Result<DlfToken> DlfEcsTokenLoader::LoadToken() {
}
if (!role_name_) {
PAIMON_ASSIGN_OR_RAISE(std::string role, Get(metadata_url_));
- TrimWhitespace(&role);
+ StringUtils::Trim(&role);
if (role.empty()) {
return Status::Invalid("DLF ECS metadata service returned an empty
role name");
}
diff --git a/src/paimon/rest/rest_api.cpp b/src/paimon/rest/rest_api.cpp
index c249ede0..a28e1991 100644
--- a/src/paimon/rest/rest_api.cpp
+++ b/src/paimon/rest/rest_api.cpp
@@ -24,6 +24,7 @@
#include "fmt/format.h"
#include "paimon/catalog_options.h"
+#include "paimon/common/utils/options_utils.h"
#include "paimon/common/utils/rapidjson_util.h"
#include "paimon/common/utils/sensitive_config_utils.h"
#include "paimon/logging.h"
@@ -61,13 +62,13 @@ RestApi::RestApi(std::unique_ptr<RestHttpClient> client,
Result<std::unique_ptr<RestApi>> RestApi::Create(const std::map<std::string,
std::string>& options,
const std::string& warehouse,
bool config_required,
const RestHttpClient::Config&
http_config) {
- auto uri_iter = options.find(CatalogOptions::URI);
- if (uri_iter == options.end() || uri_iter->second.empty()) {
+ Result<std::string> uri = OptionsUtils::GetNonEmptyValueFromMap(options,
CatalogOptions::URI);
+ if (!uri.ok()) {
return Status::Invalid(fmt::format("option '{}' must be configured for
the rest catalog",
CatalogOptions::URI));
}
PAIMON_ASSIGN_OR_RAISE(std::unique_ptr<RestHttpClient> client,
- RestHttpClient::Create(uri_iter->second,
http_config));
+ RestHttpClient::Create(uri.value(), http_config));
PAIMON_ASSIGN_OR_RAISE(std::unique_ptr<AuthProvider> auth_provider,
AuthProvider::Create(options));
diff --git a/src/paimon/rest/rest_auth.cpp b/src/paimon/rest/rest_auth.cpp
index 1af3b06a..43513b70 100644
--- a/src/paimon/rest/rest_auth.cpp
+++ b/src/paimon/rest/rest_auth.cpp
@@ -20,6 +20,7 @@
#include "fmt/format.h"
#include "paimon/catalog_options.h"
+#include "paimon/common/utils/options_utils.h"
#include "paimon/common/utils/string_utils.h"
#include "paimon/common/utils/url_utils.h"
#include "paimon/rest/dlf_auth.h"
@@ -50,22 +51,24 @@ Result<std::map<std::string, std::string>>
BearTokenAuthProvider::MergeAuthHeade
Result<std::unique_ptr<AuthProvider>> AuthProvider::Create(
const std::map<std::string, std::string>& options) {
- auto provider_iter = options.find(CatalogOptions::TOKEN_PROVIDER);
- if (provider_iter == options.end() || provider_iter->second.empty()) {
+ Result<std::string> provider_value =
+ OptionsUtils::GetNonEmptyValueFromMap(options,
CatalogOptions::TOKEN_PROVIDER);
+ if (!provider_value.ok()) {
return Status::Invalid(fmt::format("option '{}' must be configured for
the rest catalog",
CatalogOptions::TOKEN_PROVIDER));
}
// Matched leniently in lower case; other clients may match provider names
// case-sensitively, so the exact "bear" and "dlf" spellings are portable.
- std::string provider = StringUtils::ToLowerCase(provider_iter->second);
+ std::string provider = StringUtils::ToLowerCase(provider_value.value());
if (provider == "bear") {
- auto token_iter = options.find(CatalogOptions::TOKEN);
- if (token_iter == options.end() || token_iter->second.empty()) {
+ Result<std::string> token =
+ OptionsUtils::GetNonEmptyValueFromMap(options,
CatalogOptions::TOKEN);
+ if (!token.ok()) {
return Status::Invalid(
fmt::format("option '{}' must be configured for the bear token
provider",
CatalogOptions::TOKEN));
}
- return std::make_unique<BearTokenAuthProvider>(token_iter->second);
+ return std::make_unique<BearTokenAuthProvider>(token.value());
}
if (provider == "dlf") {
return DlfAuthProvider::Create(options);
diff --git a/src/paimon/rest/rest_catalog.cpp b/src/paimon/rest/rest_catalog.cpp
index c928c6a2..eb86035c 100644
--- a/src/paimon/rest/rest_catalog.cpp
+++ b/src/paimon/rest/rest_catalog.cpp
@@ -52,7 +52,7 @@ constexpr const char kPathOption[] = "path";
// `BranchManager::IsMainBranch`, which names the branch directory of a table,
stays
// case-sensitive: this normalization only decides how a table is addressed on
the server.
std::optional<std::string> NormalizeBranch(std::optional<std::string> branch) {
- if (branch && StringUtils::ToLowerCase(branch.value()) ==
Identifier::kDefaultMainBranch) {
+ if (branch && StringUtils::EqualsIgnoreCase(branch.value(),
Identifier::kDefaultMainBranch)) {
return std::nullopt;
}
return branch;
diff --git a/src/paimon/rest/rest_http_client.cpp
b/src/paimon/rest/rest_http_client.cpp
index 99b233f1..f4964bf5 100644
--- a/src/paimon/rest/rest_http_client.cpp
+++ b/src/paimon/rest/rest_http_client.cpp
@@ -228,7 +228,8 @@ std::string RestHttpClient::NormalizeUri(const std::string&
uri) {
while (!normalized.empty() && normalized.back() == '/') {
normalized.pop_back();
}
- if (normalized.rfind("http://";, 0) != 0 && normalized.rfind("https://";, 0)
!= 0) {
+ if (!StringUtils::StartsWith(normalized, "http://";) &&
+ !StringUtils::StartsWith(normalized, "https://";)) {
normalized = "http://"; + normalized;
}
return normalized;
diff --git a/src/paimon/rest/rest_util.cpp b/src/paimon/rest/rest_util.cpp
index fc8e6f69..1a746987 100644
--- a/src/paimon/rest/rest_util.cpp
+++ b/src/paimon/rest/rest_util.cpp
@@ -21,6 +21,7 @@
#include <stdexcept>
#include "fmt/format.h"
+#include "paimon/common/utils/options_utils.h"
#include "rapidjson/error/en.h"
#include "rapidjson/stringbuffer.h"
#include "rapidjson/writer.h"
@@ -29,13 +30,7 @@ namespace paimon {
std::map<std::string, std::string> RestUtil::ExtractPrefixMap(
const std::map<std::string, std::string>& options, const std::string&
prefix) {
- std::map<std::string, std::string> result;
- for (const auto& [key, value] : options) {
- if (key.size() > prefix.size() && key.compare(0, prefix.size(),
prefix) == 0) {
- result[key.substr(prefix.size())] = value;
- }
- }
- return result;
+ return OptionsUtils::FetchOptionsWithPrefix(prefix, options);
}
std::string RestUtil::ExtractRequestId(const std::map<std::string,
std::string>& headers) {
