From f88cc233aab375ea0adf625a2ee24595d4a6bfcf Mon Sep 17 00:00:00 2001
From: rui-mo <rui.mo@intel.com>
Date: Tue, 10 Jan 2023 07:28:14 +0000
Subject: [PATCH] validate cast

---
 velox/connectors/hive/HiveConnector.cpp       |  1 +
 velox/substrait/SubstraitToVeloxExpr.cpp      | 12 ----------
 .../SubstraitToVeloxPlanValidator.cpp         | 24 +++++++++++++++++++
 .../substrait/SubstraitToVeloxPlanValidator.h |  5 ++++
 4 files changed, 30 insertions(+), 12 deletions(-)

diff --git a/velox/connectors/hive/HiveConnector.cpp b/velox/connectors/hive/HiveConnector.cpp
index bdf5762ae31e..7bf68a340cd3 100644
--- a/velox/connectors/hive/HiveConnector.cpp
+++ b/velox/connectors/hive/HiveConnector.cpp
@@ -309,6 +309,7 @@ bool testFilters(
 template <TypeKind ToKind>
 velox::variant convertFromString(const std::optional<std::string>& value) {
   if (value.has_value()) {
+    // No need for casting if ToKind is VARCHAR or VARBINARY.
     if constexpr (ToKind == TypeKind::VARCHAR || ToKind == TypeKind::VARBINARY) {
       return velox::variant(value.value());
     }
diff --git a/velox/substrait/SubstraitToVeloxExpr.cpp b/velox/substrait/SubstraitToVeloxExpr.cpp
index 8fff6ff2bdbe..1e82179ae84c 100644
--- a/velox/substrait/SubstraitToVeloxExpr.cpp
+++ b/velox/substrait/SubstraitToVeloxExpr.cpp
@@ -456,18 +456,6 @@ SubstraitVeloxExprConverter::toVeloxExpr(
 
   std::vector<core::TypedExprPtr> inputs{
       toVeloxExpr(castExpr.input(), inputType)};
-
-  // TODO: refactor this part for input type validation.
-  for (const auto& input : inputs) {
-    switch (input->type()->kind()) {
-      case TypeKind::ARRAY: {
-        // Cast from array type is not supported. See CastExpr::applyCast.
-        VELOX_UNSUPPORTED("Invalid from type in casting: {}", input->type());
-      }
-      default: {
-      }
-    }
-  }
   return std::make_shared<core::CastTypedExpr>(type, inputs, nullOnFailure);
 }
 
diff --git a/velox/substrait/SubstraitToVeloxPlanValidator.cpp b/velox/substrait/SubstraitToVeloxPlanValidator.cpp
index 8bab61c95b47..e0a74849319d 100644
--- a/velox/substrait/SubstraitToVeloxPlanValidator.cpp
+++ b/velox/substrait/SubstraitToVeloxPlanValidator.cpp
@@ -94,6 +94,28 @@ bool SubstraitToVeloxPlanValidator::validateLiteral(
   return true;
 }
 
+bool SubstraitToVeloxPlanValidator::validateCast(
+    const ::substrait::Expression::Cast& castExpr,
+    const RowTypePtr& inputType) {
+  std::vector<core::TypedExprPtr> inputs{
+      exprConverter_->toVeloxExpr(castExpr.input(), inputType)};
+
+  // Casting from some types is not supported. See CastExpr::applyCast.
+  for (const auto& input : inputs) {
+    switch (input->type()->kind()) {
+      case TypeKind::ARRAY:
+      case TypeKind::MAP:
+      case TypeKind::ROW:
+      case TypeKind::VARBINARY:
+        VLOG(1) << "Invalid input type in casting: " << input->type() << ".";
+        return false;
+      default: {
+      }
+    }
+  }
+  return true;
+}
+
 bool SubstraitToVeloxPlanValidator::validateExpression(
     const ::substrait::Expression& expression,
     const RowTypePtr& inputType) {
@@ -104,6 +126,8 @@ bool SubstraitToVeloxPlanValidator::validateExpression(
       return validateScalarFunction(expression.scalar_function(), inputType);
     case ::substrait::Expression::RexTypeCase::kLiteral:
       return validateLiteral(expression.literal(), inputType);
+    case ::substrait::Expression::RexTypeCase::kCast:
+      return validateCast(expression.cast(), inputType);
     default:
       return true;
   }
diff --git a/velox/substrait/SubstraitToVeloxPlanValidator.h b/velox/substrait/SubstraitToVeloxPlanValidator.h
index de13173ebf27..5bb405afe62d 100644
--- a/velox/substrait/SubstraitToVeloxPlanValidator.h
+++ b/velox/substrait/SubstraitToVeloxPlanValidator.h
@@ -101,6 +101,11 @@ class SubstraitToVeloxPlanValidator {
       const ::substrait::Expression::ScalarFunction& scalarFunction,
       const RowTypePtr& inputType);
 
+  /// Validate Substrait Cast expression.
+  bool validateCast(
+    const ::substrait::Expression::Cast& castExpr,
+    const RowTypePtr& inputType);  
+
   /// Validate Substrait expression.
   bool validateExpression(
       const ::substrait::Expression& expression,