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


##########
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:
   Please check the java implementation, there isn't any cast from TimeLiteral 
to other Literal type in java version.



##########
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`?
   
   In java implementation, float cast to double/ decimal, while decimal not 
implemented in our project.
   ```
     static class FloatLiteral extends ComparableLiteral<Float> {
       FloatLiteral(Float value) {
         super(value);
       }
   
       @Override
       @SuppressWarnings("unchecked")
       public <T> Literal<T> to(Type type) {
         switch (type.typeId()) {
           case FLOAT:
             return (Literal<T>) this;
           case DOUBLE:
             return (Literal<T>) new DoubleLiteral(value().doubleValue());
           case DECIMAL:
             int scale = ((Types.DecimalType) type).scale();
             return (Literal<T>)
                 new DecimalLiteral(BigDecimal.valueOf(value()).setScale(scale, 
RoundingMode.HALF_UP));
           default:
             return null;
         }
       }
   ```



-- 
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