Rename and simplify casting policies. (facebookincubator#9919)

Summary:
This PR removes the old cast policies, DefaultCastPolicy, TruncateLegacyCastPolicy etc and instead introduces policies by the engine that consumes them, for e.g PrestoCastPolicy/SparkCastPolicy and LegacyCastPolicy.

PrestoCastPolicy -> equivalent to previous default cast policy , i.e doesnt support truncate or legacy casting.
SparkCastPolicy -> Doesnt support legacy casting, but supports truncate
LegacyCastPolicy -> Doesnt support truncate, but supports legacy casting.

Pull Request resolved: facebookincubator#9919

Reviewed By: kagamiori

Differential Revision: D58117116

Pulled By: kgpai

fbshipit-source-id: f312e7fd6e843b42a4017edfd468ed4cdd81f9ad

velox/expression/CMakeLists.txt

@@ -28,6 +28,7 @@ add_library(
   velox_expression
   BooleanMix.cpp
   CastExpr.cpp
+  CastHooks.cpp
   CoalesceExpr.cpp
   ConjunctExpr.cpp
   ConstantExpr.cpp

velox/expression/CastExpr-inl.h

@@ -715,30 +715,28 @@ void CastExpr::applyCastPrimitives(
   auto* resultFlatVector = result->as<FlatVector<To>>();
   auto* inputSimpleVector = input.as<SimpleVector<From>>();
 
-  if (!hooks_->truncate()) {
-    if (!hooks_->legacy()) {
-      applyToSelectedNoThrowLocal(context, rows, result, [&](int row) {
-        applyCastKernel<ToKind, FromKind, util::DefaultCastPolicy>(
-            row, context, inputSimpleVector, resultFlatVector);
-      });
-    } else {
+  switch (hooks_->getPolicy()) {
+    case LegacyCastPolicy:
       applyToSelectedNoThrowLocal(context, rows, result, [&](int row) {
         applyCastKernel<ToKind, FromKind, util::LegacyCastPolicy>(
             row, context, inputSimpleVector, resultFlatVector);
       });
-    }
-  } else {
-    if (!hooks_->legacy()) {
+      break;
+    case PrestoCastPolicy:
       applyToSelectedNoThrowLocal(context, rows, result, [&](int row) {
-        applyCastKernel<ToKind, FromKind, util::TruncateCastPolicy>(
+        applyCastKernel<ToKind, FromKind, util::PrestoCastPolicy>(
             row, context, inputSimpleVector, resultFlatVector);
       });
-    } else {
+      break;
+    case SparkCastPolicy:
       applyToSelectedNoThrowLocal(context, rows, result, [&](int row) {
-        applyCastKernel<ToKind, FromKind, util::TruncateLegacyCastPolicy>(
+        applyCastKernel<ToKind, FromKind, util::SparkCastPolicy>(
             row, context, inputSimpleVector, resultFlatVector);
       });
-    }
+      break;
+
+    default:
+      VELOX_NYI("Policy {} not yet implemented.", hooks_->getPolicy());
   }
 }
 

velox/expression/CastHooks.cpp

@@ -0,0 +1,25 @@
+/*
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "CastHooks.h"
+
+namespace facebook::velox::exec {
+
+fmt::underlying_t<PolicyType> format_as(PolicyType f) {
+  return fmt::underlying(f);
+}
+
+} // namespace facebook::velox::exec

velox/expression/CastHooks.h

@@ -21,6 +21,10 @@
 
 namespace facebook::velox::exec {
 
+enum PolicyType { LegacyCastPolicy = 1, PrestoCastPolicy, SparkCastPolicy };
+
+fmt::underlying_t<PolicyType> format_as(PolicyType f);
+
 /// This class provides cast hooks to allow different behaviors of CastExpr and
 /// SparkCastExpr. The main purpose is to create customized cast implementation
 /// by taking full usage of existing cast expression.
@@ -42,9 +46,6 @@ class CastHooks {
   // removeWhiteSpaces.
   virtual Expected<double> castStringToDouble(const StringView& data) const = 0;
 
-  // Returns whether legacy cast semantics are enabled.
-  virtual bool legacy() const = 0;
-
   // Trims all leading and trailing UTF8 whitespaces.
   virtual StringView removeWhiteSpaces(const StringView& view) const = 0;
 
@@ -53,5 +54,7 @@ class CastHooks {
 
   // Returns whether to cast to int by truncate.
   virtual bool truncate() const = 0;
+
+  virtual PolicyType getPolicy() const = 0;
 };
 } // namespace facebook::velox::exec

velox/expression/PrestoCastHooks.cpp

@@ -133,4 +133,9 @@ const TimestampToStringOptions& PrestoCastHooks::timestampToStringOptions()
     const {
   return options_;
 }
+
+PolicyType PrestoCastHooks::getPolicy() const {
+  return legacyCast_ ? PolicyType::LegacyCastPolicy
+                     : PolicyType::PrestoCastPolicy;
+}
 } // namespace facebook::velox::exec

velox/expression/PrestoCastHooks.h

@@ -42,10 +42,6 @@ class PrestoCastHooks : public CastHooks {
   // or these strings with different letter cases.
   Expected<double> castStringToDouble(const StringView& data) const override;
 
-  bool legacy() const override {
-    return legacyCast_;
-  }
-
   // Returns the input as is.
   StringView removeWhiteSpaces(const StringView& view) const override;
 
@@ -56,6 +52,8 @@ class PrestoCastHooks : public CastHooks {
     return false;
   }
 
+  PolicyType getPolicy() const override;
+
  private:
   const bool legacyCast_;
   TimestampToStringOptions options_ = {

velox/functions/sparksql/specialforms/SparkCastHooks.cpp

@@ -70,4 +70,8 @@ const TimestampToStringOptions& SparkCastHooks::timestampToStringOptions()
   };
   return options;
 }
+
+exec::PolicyType SparkCastHooks::getPolicy() const {
+  return exec::PolicyType::SparkCastPolicy;
+}
 } // namespace facebook::velox::functions::sparksql

velox/functions/sparksql/specialforms/SparkCastHooks.h

@@ -40,10 +40,6 @@ class SparkCastHooks : public exec::CastHooks {
   // strings with different letter cases to double.
   Expected<double> castStringToDouble(const StringView& data) const override;
 
-  bool legacy() const override {
-    return false;
-  }
-
   /// When casting from string to integral, floating-point, decimal, date, and
   /// timestamp types, Spark hook trims all leading and trailing UTF8
   /// whitespaces before cast.
@@ -57,5 +53,7 @@ class SparkCastHooks : public exec::CastHooks {
   bool truncate() const override {
     return true;
   }
+
+  exec::PolicyType getPolicy() const override;
 };
 } // namespace facebook::velox::functions::sparksql

velox/type/Conversions.h

@@ -31,12 +31,12 @@ DECLARE_bool(experimental_enable_legacy_cast);
 
 namespace facebook::velox::util {
 
-struct DefaultCastPolicy {
+struct PrestoCastPolicy {
   static constexpr bool truncate = false;
   static constexpr bool legacyCast = false;
 };
 
-struct TruncateCastPolicy {
+struct SparkCastPolicy {
   static constexpr bool truncate = true;
   static constexpr bool legacyCast = false;
 };
@@ -46,12 +46,7 @@ struct LegacyCastPolicy {
   static constexpr bool legacyCast = true;
 };
 
-struct TruncateLegacyCastPolicy {
-  static constexpr bool truncate = true;
-  static constexpr bool legacyCast = true;
-};
-
-template <TypeKind KIND, typename = void, typename TPolicy = DefaultCastPolicy>
+template <TypeKind KIND, typename = void, typename TPolicy = PrestoCastPolicy>
 struct Converter {
   using TTo = typename TypeTraits<KIND>::NativeType;
 

velox/type/tests/ConversionsTest.cpp

@@ -44,15 +44,13 @@ class ConversionsTest : public testing::Test {
     auto cast = [&](TFrom input) -> TTo {
       Expected<TTo> result;
       if (truncate & legacyCast) {
-        result = Converter<toTypeKind, void, TruncateLegacyCastPolicy>::tryCast(
-            input);
+        VELOX_NYI("No associated cast policy for truncate and legacy cast.");
       } else if (!truncate & legacyCast) {
         result = Converter<toTypeKind, void, LegacyCastPolicy>::tryCast(input);
       } else if (truncate & !legacyCast) {
-        result =
-            Converter<toTypeKind, void, TruncateCastPolicy>::tryCast(input);
+        result = Converter<toTypeKind, void, SparkCastPolicy>::tryCast(input);
       } else {
-        result = Converter<toTypeKind, void, DefaultCastPolicy>::tryCast(input);
+        result = Converter<toTypeKind, void, PrestoCastPolicy>::tryCast(input);
       }
 
       return result.thenOrThrow(folly::identity, [](const Status& status) {