Skip to content

Commit a4eb1f6

Browse files
wgtmacwgtmac
committed
polish some places
1 parent 1e3bbee commit a4eb1f6

10 files changed

Lines changed: 365 additions & 199 deletions

File tree

‎src/iceberg/json_serde.cc‎

Lines changed: 60 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -465,9 +465,10 @@ Result<std::unique_ptr<Type>> ListTypeFromJson(const nlohmann::json& json) {
465465
ICEBERG_ASSIGN_OR_RAISE(auto element_required,
466466
GetJsonValue<bool>(json, kElementRequired));
467467

468-
return std::make_unique<ListType>(
469-
SchemaField(element_id, std::string(ListType::kElementName),
470-
std::move(element_type), !element_required));
468+
ICEBERG_ASSIGN_OR_RAISE(auto type, ListType::Make(SchemaField(
469+
element_id, std::string(ListType::kElementName),
470+
std::move(element_type), !element_required)));
471+
return std::unique_ptr<Type>(std::move(type));
471472
}
472473

473474
Result<std::unique_ptr<Type>> MapTypeFromJson(const nlohmann::json& json) {
@@ -484,118 +485,126 @@ Result<std::unique_ptr<Type>> MapTypeFromJson(const nlohmann::json& json) {
484485
/*optional=*/false);
485486
SchemaField value_field(value_id, std::string(MapType::kValueName),
486487
std::move(value_type), !value_required);
487-
return std::make_unique<MapType>(std::move(key_field), std::move(value_field));
488+
ICEBERG_ASSIGN_OR_RAISE(auto type,
489+
MapType::Make(std::move(key_field), std::move(value_field)));
490+
return std::unique_ptr<Type>(std::move(type));
488491
}
489492

490493
} // namespace
491494

492495
Result<std::unique_ptr<Type>> TypeFromJson(const nlohmann::json& json) {
493496
if (json.is_string()) {
494-
std::string type_str = json.get<std::string>();
495-
std::string lower_type_str = StringUtils::ToLower(type_str);
496-
if (lower_type_str == "boolean") {
497+
const auto type_name = json.get<std::string>();
498+
const auto normalized_type_name = StringUtils::ToLower(type_name);
499+
if (normalized_type_name == "boolean") {
497500
return std::make_unique<BooleanType>();
498-
} else if (lower_type_str == "int") {
501+
} else if (normalized_type_name == "int") {
499502
return std::make_unique<IntType>();
500-
} else if (lower_type_str == "long") {
503+
} else if (normalized_type_name == "long") {
501504
return std::make_unique<LongType>();
502-
} else if (lower_type_str == "float") {
505+
} else if (normalized_type_name == "float") {
503506
return std::make_unique<FloatType>();
504-
} else if (lower_type_str == "double") {
507+
} else if (normalized_type_name == "double") {
505508
return std::make_unique<DoubleType>();
506-
} else if (lower_type_str == "date") {
509+
} else if (normalized_type_name == "date") {
507510
return std::make_unique<DateType>();
508-
} else if (lower_type_str == "time") {
511+
} else if (normalized_type_name == "time") {
509512
return std::make_unique<TimeType>();
510-
} else if (lower_type_str == "timestamp") {
513+
} else if (normalized_type_name == "timestamp") {
511514
return std::make_unique<TimestampType>();
512-
} else if (lower_type_str == "timestamptz") {
515+
} else if (normalized_type_name == "timestamptz") {
513516
return std::make_unique<TimestampTzType>();
514-
} else if (lower_type_str == "timestamp_ns") {
517+
} else if (normalized_type_name == "timestamp_ns") {
515518
return std::make_unique<TimestampNsType>();
516-
} else if (lower_type_str == "timestamptz_ns") {
519+
} else if (normalized_type_name == "timestamptz_ns") {
517520
return std::make_unique<TimestampTzNsType>();
518-
} else if (lower_type_str == "string") {
521+
} else if (normalized_type_name == "string") {
519522
return std::make_unique<StringType>();
520-
} else if (lower_type_str == "binary") {
523+
} else if (normalized_type_name == "binary") {
521524
return std::make_unique<BinaryType>();
522-
} else if (lower_type_str == "uuid") {
525+
} else if (normalized_type_name == "uuid") {
523526
return std::make_unique<UuidType>();
524-
} else if (lower_type_str == "unknown") {
527+
} else if (normalized_type_name == "unknown") {
525528
return std::make_unique<UnknownType>();
526-
} else if (lower_type_str == "variant") {
529+
} else if (normalized_type_name == "variant") {
527530
return std::make_unique<VariantType>();
528-
} else if (lower_type_str.starts_with("fixed")) {
529-
static const std::regex fixed_regex(R"(fixed\[\s*(\d+)\s*\])");
531+
} else if (normalized_type_name.starts_with("fixed")) {
532+
static const std::regex kFixedRegex(R"(fixed\[\s*(\d+)\s*\])");
530533
std::smatch match;
531-
if (std::regex_match(lower_type_str, match, fixed_regex)) {
534+
if (std::regex_match(normalized_type_name, match, kFixedRegex)) {
532535
ICEBERG_ASSIGN_OR_RAISE(auto length,
533536
StringUtils::ParseNumber<int32_t>(match[1].str()));
534537
return std::make_unique<FixedType>(length);
535538
}
536-
return JsonParseError("Invalid fixed type: {}", type_str);
537-
} else if (lower_type_str.starts_with("decimal")) {
538-
static const std::regex decimal_regex(R"(decimal\(\s*(\d+)\s*,\s*(\d+)\s*\))");
539+
return JsonParseError("Invalid fixed type: {}", type_name);
540+
} else if (normalized_type_name.starts_with("decimal")) {
541+
static const std::regex kDecimalRegex(R"(decimal\(\s*(\d+)\s*,\s*(\d+)\s*\))");
539542
std::smatch match;
540-
if (std::regex_match(lower_type_str, match, decimal_regex)) {
543+
if (std::regex_match(normalized_type_name, match, kDecimalRegex)) {
541544
ICEBERG_ASSIGN_OR_RAISE(auto precision,
542545
StringUtils::ParseNumber<int32_t>(match[1].str()));
543546
ICEBERG_ASSIGN_OR_RAISE(auto scale,
544547
StringUtils::ParseNumber<int32_t>(match[2].str()));
545548
return std::make_unique<DecimalType>(precision, scale);
546549
}
547-
return JsonParseError("Invalid decimal type: {}", type_str);
548-
} else if (lower_type_str.starts_with("geometry")) {
549-
static const std::regex geometry_regex(R"(geometry\s*(?:\(\s*([^)]*?)\s*\))?)",
550+
return JsonParseError("Invalid decimal type: {}", type_name);
551+
} else if (normalized_type_name.starts_with("geometry")) {
552+
static const std::regex kGeometryRegex(R"(geometry\s*(?:\(\s*([^)]*?)\s*\))?)",
550553
std::regex_constants::icase);
551554
std::smatch match;
552-
if (std::regex_match(type_str, match, geometry_regex)) {
555+
if (std::regex_match(type_name, match, kGeometryRegex)) {
553556
if (match[1].matched) {
554557
auto crs = match[1].str();
555558
if (crs.empty()) {
556-
return JsonParseError("Invalid geometry type: {}", type_str);
559+
return JsonParseError("Invalid geometry type: {}", type_name);
557560
}
558-
return std::make_unique<GeometryType>(std::move(crs));
561+
ICEBERG_ASSIGN_OR_RAISE(auto type, GeometryType::Make(std::move(crs)));
562+
return std::unique_ptr<Type>(std::move(type));
559563
}
560-
return std::make_unique<GeometryType>();
564+
ICEBERG_ASSIGN_OR_RAISE(auto type, GeometryType::Make());
565+
return std::unique_ptr<Type>(std::move(type));
561566
}
562-
return JsonParseError("Invalid geometry type: {}", type_str);
563-
} else if (lower_type_str.starts_with("geography")) {
564-
static const std::regex geography_regex(
567+
return JsonParseError("Invalid geometry type: {}", type_name);
568+
} else if (normalized_type_name.starts_with("geography")) {
569+
static const std::regex kGeographyRegex(
565570
R"(geography\s*(?:\(\s*([^,]*?)\s*(?:,\s*(\w*)\s*)?\))?)",
566571
std::regex_constants::icase);
567572
std::smatch match;
568-
if (std::regex_match(type_str, match, geography_regex)) {
573+
if (std::regex_match(type_name, match, kGeographyRegex)) {
569574
auto crs = match[1].str();
570575
if (match[1].matched && crs.empty()) {
571-
return JsonParseError("Invalid geography type: {}", type_str);
576+
return JsonParseError("Invalid geography type: {}", type_name);
572577
}
573578
if (match[2].matched) {
574579
ICEBERG_ASSIGN_OR_RAISE(auto algorithm,
575580
EdgeAlgorithmFromString(match[2].str()));
576-
return std::make_unique<GeographyType>(std::move(crs), algorithm);
581+
ICEBERG_ASSIGN_OR_RAISE(auto type,
582+
GeographyType::Make(std::move(crs), algorithm));
583+
return std::unique_ptr<Type>(std::move(type));
577584
}
578585
if (match[1].matched) {
579-
return std::make_unique<GeographyType>(std::move(crs));
586+
ICEBERG_ASSIGN_OR_RAISE(auto type, GeographyType::Make(std::move(crs)));
587+
return std::unique_ptr<Type>(std::move(type));
580588
}
581-
return std::make_unique<GeographyType>();
589+
ICEBERG_ASSIGN_OR_RAISE(auto type, GeographyType::Make());
590+
return std::unique_ptr<Type>(std::move(type));
582591
}
583-
return JsonParseError("Invalid geography type: {}", type_str);
592+
return JsonParseError("Invalid geography type: {}", type_name);
584593
} else {
585-
return JsonParseError("Cannot parse type string: {}", type_str);
594+
return JsonParseError("Cannot parse type string: {}", type_name);
586595
}
587596
}
588597

589598
// For complex types like struct, list, and map
590-
ICEBERG_ASSIGN_OR_RAISE(auto type_str, GetJsonValue<std::string>(json, kType));
591-
if (type_str == kStruct) {
599+
ICEBERG_ASSIGN_OR_RAISE(auto complex_type_name, GetJsonValue<std::string>(json, kType));
600+
if (complex_type_name == kStruct) {
592601
return StructTypeFromJson(json);
593-
} else if (type_str == kList) {
602+
} else if (complex_type_name == kList) {
594603
return ListTypeFromJson(json);
595-
} else if (type_str == kMap) {
604+
} else if (complex_type_name == kMap) {
596605
return MapTypeFromJson(json);
597606
} else {
598-
return JsonParseError("Unknown complex type: {}", type_str);
607+
return JsonParseError("Unknown complex type: {}", complex_type_name);
599608
}
600609
}
601610

‎src/iceberg/metrics_config.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,7 @@ Result<std::unordered_set<int32_t>> MetricsConfig::LimitFieldIds(const Schema& s
197197
Status Visit(const Type& type) {
198198
if (type.is_nested()) {
199199
return VisitNested(internal::checked_cast<const NestedType&>(type));
200-
} else if (type.type_id() == TypeId::kVariant) {
200+
} else if (type.is_variant()) {
201201
return {};
202202
} else {
203203
return VisitPrimitive(internal::checked_cast<const PrimitiveType&>(type));

0 commit comments

Comments
 (0)