Skip to content

Commit da67168

Browse files
committed
fix comments
1 parent fe31896 commit da67168

6 files changed

Lines changed: 40 additions & 9 deletions

File tree

src/paimon/common/utils/binary_row_partition_computer_test.cpp

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -251,20 +251,27 @@ TEST(BinaryRowPartitionComputerTest, TestNullOrWhitespaceOnlyStr) {
251251
arrow::field("f0", arrow::utf8()),
252252
arrow::field("f1", arrow::utf8()),
253253
arrow::field("f2", arrow::utf8()),
254+
arrow::field("f3", arrow::utf8()),
254255
};
255256

256257
auto schema = arrow::schema(fields);
257-
std::vector<std::string> partition_keys = {"f0", "f1", "f2"};
258+
std::vector<std::string> partition_keys = {"f0", "f1", "f2", "f3"};
258259
ASSERT_OK_AND_ASSIGN(
259260
std::unique_ptr<BinaryRowPartitionComputer> computer,
260261
BinaryRowPartitionComputer::Create(partition_keys, schema, "__DEFAULT_PARTITION__",
261262
/*legacy_partition_name_enabled=*/true, pool));
262263

263-
ASSERT_OK_AND_ASSIGN(auto partition_key_values,
264-
computer->GeneratePartitionVector(BinaryRowGenerator::GenerateRow(
265-
{std::string(" "), std::string(""), std::string("ab ")}, pool.get())));
264+
ASSERT_OK_AND_ASSIGN(
265+
auto partition_key_values,
266+
computer->GeneratePartitionVector(BinaryRowGenerator::GenerateRow(
267+
{std::string(" "), std::string(""), std::string("ab "), std::string(u8"\u3000\u2000")},
268+
pool.get())));
266269
std::vector<std::pair<std::string, std::string>> expected = {
267-
{"f0", "__DEFAULT_PARTITION__"}, {"f1", "__DEFAULT_PARTITION__"}, {"f2", "ab "}};
270+
{"f0", "__DEFAULT_PARTITION__"},
271+
{"f1", "__DEFAULT_PARTITION__"},
272+
{"f2", "ab "},
273+
{"f3", "__DEFAULT_PARTITION__"},
274+
};
268275
ASSERT_EQ(partition_key_values, expected);
269276
}
270277

src/paimon/common/utils/string_utils.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,10 @@ bool StringUtils::IsNullOrWhitespaceOnly(const std::string& str) {
141141
return IsBlank(str);
142142
}
143143

144+
bool StringUtils::IsEmptyAfterTrim(std::string_view str) {
145+
return std::all_of(str.begin(), str.end(), [](unsigned char c) { return IsTrimCharacter(c); });
146+
}
147+
144148
void StringUtils::Trim(std::string* str) {
145149
auto first = std::find_if_not(str->begin(), str->end(),
146150
[](unsigned char c) { return IsTrimCharacter(c); });

src/paimon/common/utils/string_utils.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,9 @@ class PAIMON_EXPORT StringUtils {
103103

104104
static bool IsNullOrWhitespaceOnly(const std::string& str);
105105

106+
/// Returns true if Java String::trim would produce an empty string.
107+
static bool IsEmptyAfterTrim(std::string_view str);
108+
106109
static void Trim(std::string* str);
107110

108111
static std::string ToLowerCase(const std::string& str);

src/paimon/common/utils/string_utils_test.cpp

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,19 @@ TEST_F(StringUtilsTest, TestIsBlank) {
225225
ASSERT_FALSE(StringUtils::IsBlank(" user1 "));
226226
ASSERT_FALSE(StringUtils::IsBlank(u8"\u00a0"));
227227
ASSERT_FALSE(StringUtils::IsBlank(std::string("\xc0\x80", 2)));
228+
229+
// Non-breaking or otherwise excluded by Character.isWhitespace.
230+
ASSERT_FALSE(StringUtils::IsBlank(u8"\u2007")); // FIGURE SPACE
231+
ASSERT_FALSE(StringUtils::IsBlank(u8"\u202f")); // NARROW NO-BREAK SPACE
232+
ASSERT_FALSE(StringUtils::IsBlank(u8"\u0085")); // NEL
233+
ASSERT_FALSE(StringUtils::IsBlank(u8"\u180e")); // Not whitespace since Java 8
234+
}
235+
236+
TEST_F(StringUtilsTest, TestIsEmptyAfterTrim) {
237+
ASSERT_TRUE(StringUtils::IsEmptyAfterTrim(""));
238+
ASSERT_TRUE(StringUtils::IsEmptyAfterTrim(" \t\x1c"));
239+
ASSERT_FALSE(StringUtils::IsEmptyAfterTrim(" a "));
240+
ASSERT_FALSE(StringUtils::IsEmptyAfterTrim(u8"\u3000"));
228241
}
229242

230243
TEST_F(StringUtilsTest, TestToLowerCase) {

src/paimon/core/index/pk/primary_key_index_definitions.cpp

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,10 @@ Result<std::map<std::string, std::string>> SortedIndexOptions(
102102
std::string option_key =
103103
fmt::format("{}{}.{}.index.options", kFieldScopedPrefix, column, option_family);
104104
auto iter = table_options.find(option_key);
105-
if (iter == table_options.end() || StringUtils::IsNullOrWhitespaceOnly(iter->second)) {
105+
if (iter == table_options.end()) {
106+
return resolved;
107+
}
108+
if (StringUtils::IsEmptyAfterTrim(iter->second)) {
106109
return resolved;
107110
}
108111

@@ -113,11 +116,13 @@ Result<std::map<std::string, std::string>> SortedIndexOptions(
113116
fmt::format("{} must be a JSON object of option key-value pairs.", option_key));
114117
}
115118
for (auto member = document.MemberBegin(); member != document.MemberEnd(); ++member) {
116-
if (!member->name.IsString() ||
117-
StringUtils::IsNullOrWhitespaceOnly(member->name.GetString())) {
119+
if (!member->name.IsString()) {
118120
return Status::Invalid(fmt::format("{} contains an empty option key.", option_key));
119121
}
120122
std::string key = member->name.GetString();
123+
if (StringUtils::IsEmptyAfterTrim(key)) {
124+
return Status::Invalid(fmt::format("{} contains an empty option key.", option_key));
125+
}
121126
if (member->value.IsNull()) {
122127
return Status::Invalid(
123128
fmt::format("{} value for key {} must not be null.", option_key, key));

test/inte/variant_table_inte_test.cpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,6 @@ class VariantTableInteTest : public ::testing::Test {
179179
double admission_ratio,
180180
double retention_ratio) const {
181181
return {
182-
183182
{Options::FILE_FORMAT, "parquet"},
184183
{Options::BUCKET, "-1"},
185184
{Options::WRITE_ONLY, "true"},

0 commit comments

Comments
 (0)