nullccxsy commented on code in PR #206:
URL: https://github.com/apache/iceberg-cpp/pull/206#discussion_r2342815051


##########
src/iceberg/expression/literal.cc:
##########
@@ -109,17 +172,230 @@ Result<Literal> LiteralCaster::CastFromLong(
 Result<Literal> LiteralCaster::CastFromFloat(
     const Literal& literal, const std::shared_ptr<PrimitiveType>& target_type) 
{
   auto float_val = std::get<float>(literal.value_);
-  auto target_type_id = target_type->type_id();
 
-  switch (target_type_id) {
+  switch (target_type->type_id()) {
     case TypeId::kDouble:
       return Literal::Double(static_cast<double>(float_val));
+    // TODO(Li Feiyang): Implement cast from Float to decimal
     default:
       return NotSupported("Cast from Float to {} is not supported",
                           target_type->ToString());
   }
 }
 
+Result<Literal> LiteralCaster::CastFromDouble(
+    const Literal& literal, const std::shared_ptr<PrimitiveType>& target_type) 
{
+  auto double_val = std::get<double>(literal.value_);
+
+  switch (target_type->type_id()) {
+    case TypeId::kFloat: {
+      if (double_val > static_cast<double>(std::numeric_limits<float>::max())) 
{
+        return AboveMaxLiteral(target_type);
+      }
+      if (double_val < 
static_cast<double>(std::numeric_limits<float>::lowest())) {
+        return BelowMinLiteral(target_type);
+      }
+      return Literal::Float(static_cast<float>(double_val));
+    }
+    default:
+      return NotSupported("Cast from Double to {} is not supported",
+                          target_type->ToString());
+  }
+}
+
+Result<Literal> LiteralCaster::CastFromString(
+    const Literal& literal, const std::shared_ptr<PrimitiveType>& target_type) 
{
+  const auto& str_val = std::get<std::string>(literal.value_);
+  std::istringstream in{str_val};
+  std::tm tm = {};
+
+  switch (target_type->type_id()) {
+    case TypeId::kDate: {
+      // Parse "YYYY-MM-DD" into days since 1970-01-01 epoch.
+      in >> std::get_time(&tm, "%Y-%m-%d");
+
+      if (in.fail() || tm.tm_mday == 0 || in.peek() != EOF) {
+        return NotSupported("Failed to parse '{}' as a valid Date (expected 
YYYY-MM-DD)",
+                            str_val);
+      }
+
+      auto time_point = 
std::chrono::system_clock::from_time_t(timegm_custom(&tm));
+      auto days_since_epoch = 
std::chrono::floor<std::chrono::days>(time_point);
+      return Literal::Date(
+          static_cast<int32_t>(days_since_epoch.time_since_epoch().count()));
+    }
+
+    case TypeId::kTime: {
+      // Parse "HH:MM:SS.ffffff" into microseconds since midnight.
+      in >> std::get_time(&tm, "%H:%M:%S");
+
+      if (in.fail()) {
+        return NotSupported(
+            "Failed to parse '{}' as a valid Time (expected HH:MM:SS.ffffff)", 
str_val);
+      }
+
+      int64_t total_micros =
+          (tm.tm_hour * 3600LL + tm.tm_min * 60LL + tm.tm_sec) * 1000000LL;
+
+      if (in.peek() == '.') {
+        in.ignore();
+        std::string fractional_str;
+        char c;
+        while (in.get(c) && isdigit(c)) {
+          fractional_str += c;
+        }
+        if (in) {
+          in.unget();
+        }
+
+        if (fractional_str.length() > 6) {
+          fractional_str.resize(6);
+        }
+        try {
+          if (!fractional_str.empty()) {
+            fractional_str.append(6 - fractional_str.length(), '0');
+            total_micros += std::stoll(fractional_str);
+          }
+        } catch (const std::exception&) {
+          return NotSupported("Failed to parse fractional part of Time '{}'", 
str_val);
+        }
+      }
+
+      if (in.peek() != EOF) {
+        return NotSupported("Unconsumed characters found after parsing Time 
'{}'",
+                            str_val);
+      }
+
+      return Literal::Time(total_micros);
+    }
+
+    case TypeId::kTimestamp:
+    case TypeId::kTimestampTz: {
+      // Parse "YYYY-MM-DDTHH:MM:SS.ffffff" and optional 'Z'
+      in >> std::get_time(&tm, "%Y-%m-%dT%H:%M:%S");
+
+      if (in.fail()) {
+        return NotSupported(
+            "Failed to parse '{}' as a valid Timestamp (expected 
YYYY-MM-DDTHH:MM:SS...)",
+            str_val);
+      }
+
+      auto seconds_since_epoch = timegm_custom(&tm);
+      int64_t total_micros = seconds_since_epoch * 1000000LL;
+
+      if (in.peek() == '.') {
+        in.ignore();
+        std::string fractional_str;
+        char c;
+        while (in.get(c) && isdigit(c)) {
+          fractional_str += c;
+        }
+        if (in) {
+          in.unget();
+        }
+
+        if (fractional_str.length() > 6) {
+          fractional_str.resize(6);
+        }
+        try {
+          if (!fractional_str.empty()) {
+            fractional_str.append(6 - fractional_str.length(), '0');
+            total_micros += std::stoll(fractional_str);
+          }
+        } catch (const std::exception&) {
+          return NotSupported("Failed to parse fractional part of Timestamp 
'{}'",
+                              str_val);
+        }
+      }
+
+      if (target_type->type_id() == TypeId::kTimestampTz) {
+        // NOTE: This implementation DOES NOT support timezone offsets like
+        // '+08:00' or '-07:00'. It only supports the UTC designator 'Z'.
+        if (in.peek() == 'Z') {
+          in.ignore();  // Consume 'Z'
+        }
+      }
+
+      if (in.peek() != EOF) {
+        return NotSupported("Unconsumed characters found after parsing 
Timestamp '{}'",
+                            str_val);
+      }
+
+      if (target_type->type_id() == TypeId::kTimestamp) {
+        return Literal::Timestamp(total_micros);
+      } else {
+        return Literal::TimestampTz(total_micros);
+      }
+    }
+      // TODO(Li Feiyang): Implement cast from String to uuid and decimal
+
+    default:
+      return NotSupported("Cast from String to {} is not supported",
+                          target_type->ToString());
+  }
+}
+
+Result<Literal> LiteralCaster::CastFromTimestamp(
+    const Literal& literal, const std::shared_ptr<PrimitiveType>& target_type) 
{
+  auto timestamp_val = std::get<int64_t>(literal.value_);
+
+  switch (target_type->type_id()) {
+    case TypeId::kDate:
+      return Literal::Date(MicrosToDays(timestamp_val));
+    case TypeId::kTimestampTz:
+      return Literal::TimestampTz(timestamp_val);
+    default:
+      return NotSupported("Cast from Timestamp to {} is not supported",
+                          target_type->ToString());
+  }
+}
+
+Result<Literal> LiteralCaster::CastFromTimestampTz(
+    const Literal& literal, const std::shared_ptr<PrimitiveType>& target_type) 
{
+  auto micros = std::get<int64_t>(literal.value_);
+
+  switch (target_type->type_id()) {
+    case TypeId::kDate:
+      return Literal::Date(MicrosToDays(micros));
+    case TypeId::kTimestamp:
+      return Literal::Timestamp(micros);
+    default:
+      return NotSupported("Cast from TimestampTz to {} is not supported",
+                          target_type->ToString());
+  }
+}
+
+Result<Literal> LiteralCaster::CastFromBinary(
+    const Literal& literal, const std::shared_ptr<PrimitiveType>& target_type) 
{
+  auto binary_val = std::get<std::vector<uint8_t>>(literal.value_);
+  switch (target_type->type_id()) {
+    case TypeId::kFixed: {
+      auto target_fixed_type = 
std::dynamic_pointer_cast<FixedType>(target_type);
+      if (binary_val.size() == target_fixed_type->length()) {
+        return Literal::Fixed(binary_val);

Review Comment:
   There we can use `std::move(binary_val)` or use `const auto& binary_val` in 
line259



##########
src/iceberg/expression/literal.cc:
##########
@@ -109,17 +172,230 @@ Result<Literal> LiteralCaster::CastFromLong(
 Result<Literal> LiteralCaster::CastFromFloat(
     const Literal& literal, const std::shared_ptr<PrimitiveType>& target_type) 
{
   auto float_val = std::get<float>(literal.value_);
-  auto target_type_id = target_type->type_id();
 
-  switch (target_type_id) {
+  switch (target_type->type_id()) {
     case TypeId::kDouble:
       return Literal::Double(static_cast<double>(float_val));
+    // TODO(Li Feiyang): Implement cast from Float to decimal
     default:
       return NotSupported("Cast from Float to {} is not supported",
                           target_type->ToString());
   }
 }
 
+Result<Literal> LiteralCaster::CastFromDouble(
+    const Literal& literal, const std::shared_ptr<PrimitiveType>& target_type) 
{
+  auto double_val = std::get<double>(literal.value_);
+
+  switch (target_type->type_id()) {
+    case TypeId::kFloat: {
+      if (double_val > static_cast<double>(std::numeric_limits<float>::max())) 
{
+        return AboveMaxLiteral(target_type);
+      }
+      if (double_val < 
static_cast<double>(std::numeric_limits<float>::lowest())) {
+        return BelowMinLiteral(target_type);
+      }
+      return Literal::Float(static_cast<float>(double_val));
+    }
+    default:
+      return NotSupported("Cast from Double to {} is not supported",
+                          target_type->ToString());
+  }
+}
+
+Result<Literal> LiteralCaster::CastFromString(
+    const Literal& literal, const std::shared_ptr<PrimitiveType>& target_type) 
{
+  const auto& str_val = std::get<std::string>(literal.value_);
+  std::istringstream in{str_val};
+  std::tm tm = {};
+
+  switch (target_type->type_id()) {
+    case TypeId::kDate: {
+      // Parse "YYYY-MM-DD" into days since 1970-01-01 epoch.
+      in >> std::get_time(&tm, "%Y-%m-%d");
+
+      if (in.fail() || tm.tm_mday == 0 || in.peek() != EOF) {
+        return NotSupported("Failed to parse '{}' as a valid Date (expected 
YYYY-MM-DD)",
+                            str_val);
+      }
+
+      auto time_point = 
std::chrono::system_clock::from_time_t(timegm_custom(&tm));
+      auto days_since_epoch = 
std::chrono::floor<std::chrono::days>(time_point);
+      return Literal::Date(
+          static_cast<int32_t>(days_since_epoch.time_since_epoch().count()));
+    }
+
+    case TypeId::kTime: {
+      // Parse "HH:MM:SS.ffffff" into microseconds since midnight.
+      in >> std::get_time(&tm, "%H:%M:%S");
+
+      if (in.fail()) {
+        return NotSupported(
+            "Failed to parse '{}' as a valid Time (expected HH:MM:SS.ffffff)", 
str_val);
+      }
+
+      int64_t total_micros =
+          (tm.tm_hour * 3600LL + tm.tm_min * 60LL + tm.tm_sec) * 1000000LL;
+
+      if (in.peek() == '.') {
+        in.ignore();
+        std::string fractional_str;
+        char c;
+        while (in.get(c) && isdigit(c)) {
+          fractional_str += c;
+        }
+        if (in) {
+          in.unget();
+        }
+
+        if (fractional_str.length() > 6) {
+          fractional_str.resize(6);
+        }
+        try {
+          if (!fractional_str.empty()) {
+            fractional_str.append(6 - fractional_str.length(), '0');
+            total_micros += std::stoll(fractional_str);
+          }
+        } catch (const std::exception&) {
+          return NotSupported("Failed to parse fractional part of Time '{}'", 
str_val);
+        }
+      }
+
+      if (in.peek() != EOF) {
+        return NotSupported("Unconsumed characters found after parsing Time 
'{}'",
+                            str_val);
+      }
+
+      return Literal::Time(total_micros);
+    }
+
+    case TypeId::kTimestamp:
+    case TypeId::kTimestampTz: {
+      // Parse "YYYY-MM-DDTHH:MM:SS.ffffff" and optional 'Z'
+      in >> std::get_time(&tm, "%Y-%m-%dT%H:%M:%S");
+
+      if (in.fail()) {
+        return NotSupported(
+            "Failed to parse '{}' as a valid Timestamp (expected 
YYYY-MM-DDTHH:MM:SS...)",
+            str_val);
+      }
+
+      auto seconds_since_epoch = timegm_custom(&tm);
+      int64_t total_micros = seconds_since_epoch * 1000000LL;
+
+      if (in.peek() == '.') {
+        in.ignore();
+        std::string fractional_str;
+        char c;
+        while (in.get(c) && isdigit(c)) {
+          fractional_str += c;
+        }
+        if (in) {
+          in.unget();
+        }
+
+        if (fractional_str.length() > 6) {
+          fractional_str.resize(6);
+        }
+        try {
+          if (!fractional_str.empty()) {
+            fractional_str.append(6 - fractional_str.length(), '0');
+            total_micros += std::stoll(fractional_str);
+          }
+        } catch (const std::exception&) {
+          return NotSupported("Failed to parse fractional part of Timestamp 
'{}'",
+                              str_val);
+        }
+      }
+
+      if (target_type->type_id() == TypeId::kTimestampTz) {
+        // NOTE: This implementation DOES NOT support timezone offsets like
+        // '+08:00' or '-07:00'. It only supports the UTC designator 'Z'.
+        if (in.peek() == 'Z') {
+          in.ignore();  // Consume 'Z'
+        }
+      }
+
+      if (in.peek() != EOF) {
+        return NotSupported("Unconsumed characters found after parsing 
Timestamp '{}'",
+                            str_val);
+      }
+
+      if (target_type->type_id() == TypeId::kTimestamp) {
+        return Literal::Timestamp(total_micros);
+      } else {
+        return Literal::TimestampTz(total_micros);
+      }
+    }
+      // TODO(Li Feiyang): Implement cast from String to uuid and decimal
+
+    default:
+      return NotSupported("Cast from String to {} is not supported",
+                          target_type->ToString());
+  }
+}
+
+Result<Literal> LiteralCaster::CastFromTimestamp(
+    const Literal& literal, const std::shared_ptr<PrimitiveType>& target_type) 
{
+  auto timestamp_val = std::get<int64_t>(literal.value_);
+
+  switch (target_type->type_id()) {
+    case TypeId::kDate:
+      return Literal::Date(MicrosToDays(timestamp_val));
+    case TypeId::kTimestampTz:
+      return Literal::TimestampTz(timestamp_val);
+    default:
+      return NotSupported("Cast from Timestamp to {} is not supported",
+                          target_type->ToString());
+  }
+}
+
+Result<Literal> LiteralCaster::CastFromTimestampTz(
+    const Literal& literal, const std::shared_ptr<PrimitiveType>& target_type) 
{
+  auto micros = std::get<int64_t>(literal.value_);
+
+  switch (target_type->type_id()) {
+    case TypeId::kDate:
+      return Literal::Date(MicrosToDays(micros));
+    case TypeId::kTimestamp:
+      return Literal::Timestamp(micros);
+    default:
+      return NotSupported("Cast from TimestampTz to {} is not supported",
+                          target_type->ToString());
+  }
+}
+
+Result<Literal> LiteralCaster::CastFromBinary(
+    const Literal& literal, const std::shared_ptr<PrimitiveType>& target_type) 
{
+  auto binary_val = std::get<std::vector<uint8_t>>(literal.value_);
+  switch (target_type->type_id()) {
+    case TypeId::kFixed: {
+      auto target_fixed_type = 
std::dynamic_pointer_cast<FixedType>(target_type);
+      if (binary_val.size() == target_fixed_type->length()) {
+        return Literal::Fixed(binary_val);
+      }
+      return NotSupported("Cannot cast Binary with length {} to Fixed({})",
+                          binary_val.size(), target_fixed_type->length());
+    }
+    default:
+      return NotSupported("Cast from Binary to {} is not supported",
+                          target_type->ToString());
+  }
+}
+
+Result<Literal> LiteralCaster::CastFromFixed(
+    const Literal& literal, const std::shared_ptr<PrimitiveType>& target_type) 
{
+  const auto& fixed_val = std::get<std::vector<uint8_t>>(literal.value_);
+
+  switch (target_type->type_id()) {
+    case TypeId::kBinary:
+      return Literal::Binary(fixed_val);

Review Comment:
   There we have know target_type is FixedType, so may we can use `static_cast`



##########
src/iceberg/expression/literal.cc:
##########
@@ -109,17 +172,230 @@ Result<Literal> LiteralCaster::CastFromLong(
 Result<Literal> LiteralCaster::CastFromFloat(
     const Literal& literal, const std::shared_ptr<PrimitiveType>& target_type) 
{
   auto float_val = std::get<float>(literal.value_);
-  auto target_type_id = target_type->type_id();
 
-  switch (target_type_id) {
+  switch (target_type->type_id()) {

Review Comment:
   Why there don't cast to `int`?



##########
src/iceberg/expression/literal.cc:
##########
@@ -109,17 +172,230 @@ Result<Literal> LiteralCaster::CastFromLong(
 Result<Literal> LiteralCaster::CastFromFloat(
     const Literal& literal, const std::shared_ptr<PrimitiveType>& target_type) 
{
   auto float_val = std::get<float>(literal.value_);
-  auto target_type_id = target_type->type_id();
 
-  switch (target_type_id) {
+  switch (target_type->type_id()) {
     case TypeId::kDouble:
       return Literal::Double(static_cast<double>(float_val));
+    // TODO(Li Feiyang): Implement cast from Float to decimal
     default:
       return NotSupported("Cast from Float to {} is not supported",
                           target_type->ToString());
   }
 }
 
+Result<Literal> LiteralCaster::CastFromDouble(
+    const Literal& literal, const std::shared_ptr<PrimitiveType>& target_type) 
{
+  auto double_val = std::get<double>(literal.value_);
+
+  switch (target_type->type_id()) {
+    case TypeId::kFloat: {
+      if (double_val > static_cast<double>(std::numeric_limits<float>::max())) 
{
+        return AboveMaxLiteral(target_type);
+      }
+      if (double_val < 
static_cast<double>(std::numeric_limits<float>::lowest())) {
+        return BelowMinLiteral(target_type);
+      }
+      return Literal::Float(static_cast<float>(double_val));
+    }
+    default:
+      return NotSupported("Cast from Double to {} is not supported",
+                          target_type->ToString());
+  }
+}
+
+Result<Literal> LiteralCaster::CastFromString(
+    const Literal& literal, const std::shared_ptr<PrimitiveType>& target_type) 
{
+  const auto& str_val = std::get<std::string>(literal.value_);
+  std::istringstream in{str_val};
+  std::tm tm = {};
+
+  switch (target_type->type_id()) {
+    case TypeId::kDate: {
+      // Parse "YYYY-MM-DD" into days since 1970-01-01 epoch.
+      in >> std::get_time(&tm, "%Y-%m-%d");
+
+      if (in.fail() || tm.tm_mday == 0 || in.peek() != EOF) {
+        return NotSupported("Failed to parse '{}' as a valid Date (expected 
YYYY-MM-DD)",
+                            str_val);
+      }
+
+      auto time_point = 
std::chrono::system_clock::from_time_t(timegm_custom(&tm));
+      auto days_since_epoch = 
std::chrono::floor<std::chrono::days>(time_point);
+      return Literal::Date(
+          static_cast<int32_t>(days_since_epoch.time_since_epoch().count()));
+    }
+
+    case TypeId::kTime: {
+      // Parse "HH:MM:SS.ffffff" into microseconds since midnight.
+      in >> std::get_time(&tm, "%H:%M:%S");
+
+      if (in.fail()) {
+        return NotSupported(
+            "Failed to parse '{}' as a valid Time (expected HH:MM:SS.ffffff)", 
str_val);
+      }
+
+      int64_t total_micros =
+          (tm.tm_hour * 3600LL + tm.tm_min * 60LL + tm.tm_sec) * 1000000LL;
+
+      if (in.peek() == '.') {

Review Comment:
   These cods is same with the line286-309, I think we can add a function to 
reduce duplicate code.



##########
src/iceberg/expression/literal.cc:
##########
@@ -343,10 +632,18 @@ Result<Literal> LiteralCaster::CastTo(const Literal& 
literal,
     case TypeId::kFloat:
       return CastFromFloat(literal, target_type);
     case TypeId::kDouble:
-    case TypeId::kBoolean:
+      return CastFromDouble(literal, target_type);
     case TypeId::kString:
+      return CastFromString(literal, target_type);
     case TypeId::kBinary:
-      break;
+      return CastFromBinary(literal, target_type);
+    case TypeId::kFixed:
+      return CastFromFixed(literal, target_type);
+    case TypeId::kTimestamp:

Review Comment:
   Why there don't have `CastFromTime()`?



-- 
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: issues-unsubscr...@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org

Reply via email to