Skip to content

Commit 4a5759e

Browse files
authored
fix: make enum option in CoreOptions parsing case-insensitive (alibaba#136)
1 parent 37ba043 commit 4a5759e

2 files changed

Lines changed: 35 additions & 11 deletions

File tree

src/paimon/core/core_options.cpp

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ class ConfigParser {
150150
Status ParseSortOrder(SortOrder* sort_order) const {
151151
auto iter = config_map_.find(Options::SEQUENCE_FIELD_SORT_ORDER);
152152
if (iter != config_map_.end()) {
153-
const auto& str = iter->second;
153+
std::string str = StringUtils::ToLowerCase(iter->second);
154154
if (str == "ascending") {
155155
*sort_order = SortOrder::ASCENDING;
156156
} else if (str == "descending") {
@@ -166,7 +166,7 @@ class ConfigParser {
166166
Status ParseSortEngine(SortEngine* sort_engine) const {
167167
auto iter = config_map_.find(Options::SORT_ENGINE);
168168
if (iter != config_map_.end()) {
169-
const auto& str = iter->second;
169+
std::string str = StringUtils::ToLowerCase(iter->second);
170170
if (str == "min-heap") {
171171
*sort_engine = SortEngine::MIN_HEAP;
172172
} else if (str == "loser-tree") {
@@ -182,7 +182,7 @@ class ConfigParser {
182182
Status ParseMergeEngine(MergeEngine* merge_engine) const {
183183
auto iter = config_map_.find(Options::MERGE_ENGINE);
184184
if (iter != config_map_.end()) {
185-
const auto& str = iter->second;
185+
std::string str = StringUtils::ToLowerCase(iter->second);
186186
if (str == "deduplicate") {
187187
*merge_engine = MergeEngine::DEDUPLICATE;
188188
} else if (str == "partial-update") {
@@ -202,7 +202,7 @@ class ConfigParser {
202202
Status ParseChangelogProducer(ChangelogProducer* changelog_producer) const {
203203
auto iter = config_map_.find(Options::CHANGELOG_PRODUCER);
204204
if (iter != config_map_.end()) {
205-
const auto& str = iter->second;
205+
std::string str = StringUtils::ToLowerCase(iter->second);
206206
if (str == "none") {
207207
*changelog_producer = ChangelogProducer::NONE;
208208
} else if (str == "input") {
@@ -222,7 +222,7 @@ class ConfigParser {
222222
Status ParseExternalPathStrategy(ExternalPathStrategy* external_path_strategy) const {
223223
auto iter = config_map_.find(Options::DATA_FILE_EXTERNAL_PATHS_STRATEGY);
224224
if (iter != config_map_.end()) {
225-
const auto& str = iter->second;
225+
std::string str = StringUtils::ToLowerCase(iter->second);
226226
if (str == "none") {
227227
*external_path_strategy = ExternalPathStrategy::NONE;
228228
} else if (str == "specific-fs") {
@@ -236,6 +236,16 @@ class ConfigParser {
236236
return Status::OK();
237237
}
238238

239+
// Parse StartupMode
240+
Status ParseStartupMode(StartupMode* startup_mode) const {
241+
auto iter = config_map_.find(Options::SCAN_MODE);
242+
if (iter != config_map_.end()) {
243+
std::string str = StringUtils::ToLowerCase(iter->second);
244+
PAIMON_ASSIGN_OR_RAISE(*startup_mode, StartupMode::FromString(str));
245+
}
246+
return Status::OK();
247+
}
248+
239249
private:
240250
const std::map<std::string, std::string> config_map_;
241251
};
@@ -361,12 +371,7 @@ Result<CoreOptions> CoreOptions::FromMap(
361371
&impl->file_system));
362372

363373
// Parse startup mode
364-
std::string startup_mode_str;
365-
if (options_map.find(Options::SCAN_MODE) != options_map.end()) {
366-
PAIMON_RETURN_NOT_OK(parser.ParseString(Options::SCAN_MODE, &startup_mode_str));
367-
PAIMON_ASSIGN_OR_RAISE(auto startup_mode, StartupMode::FromString(startup_mode_str));
368-
impl->startup_mode = startup_mode;
369-
}
374+
PAIMON_RETURN_NOT_OK(parser.ParseStartupMode(&impl->startup_mode));
370375

371376
// Special handling for ExpireConfig
372377
int32_t snapshot_num_retain_min = 10;

src/paimon/core/core_options_test.cpp

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -336,4 +336,23 @@ TEST(CoreOptionsTest, TestFileSystem) {
336336
typed_fs->GetRealFileSystem("oss:///tmp/").value_or(nullptr)));
337337
}
338338
}
339+
340+
TEST(CoreOptionsTest, TestNormalizeValueInCoreOption) {
341+
std::map<std::string, std::string> options = {
342+
{Options::SEQUENCE_FIELD_SORT_ORDER, "ASCENDING"},
343+
{Options::SORT_ENGINE, "MIN-heap"},
344+
{Options::MERGE_ENGINE, "first-ROW"},
345+
{Options::CHANGELOG_PRODUCER, "LOOKUP"},
346+
{Options::DATA_FILE_EXTERNAL_PATHS_STRATEGY, "ROUND-ROBIN"},
347+
{Options::SCAN_MODE, "DEFAULT"},
348+
};
349+
ASSERT_OK_AND_ASSIGN(CoreOptions core_options, CoreOptions::FromMap(options));
350+
351+
ASSERT_EQ(StartupMode::LatestFull(), core_options.GetStartupMode());
352+
ASSERT_EQ(ExternalPathStrategy::ROUND_ROBIN, core_options.GetExternalPathStrategy());
353+
ASSERT_EQ(ChangelogProducer::LOOKUP, core_options.GetChangelogProducer());
354+
ASSERT_EQ(MergeEngine::FIRST_ROW, core_options.GetMergeEngine());
355+
ASSERT_EQ(SortEngine::MIN_HEAP, core_options.GetSortEngine());
356+
ASSERT_TRUE(core_options.SequenceFieldSortOrderIsAscending());
357+
}
339358
} // namespace paimon::test

0 commit comments

Comments
 (0)