Fix comments

PHILO-HE · Feb 16, 2024 · 240223f · 240223f
1 parent 214198b
commit 240223f
Show file tree

Hide file tree

Showing 7 changed files with 20 additions and 64 deletions.
diff --git a/velox/exec/tests/utils/QueryAssertions.cpp b/velox/exec/tests/utils/QueryAssertions.cpp
@@ -274,14 +274,6 @@ variant variantAt<TypeKind::TIMESTAMP>(
       dataChunk->GetValue(column, row).GetValue<::duckdb::timestamp_t>()));
 }
 
-template <>
-variant variantAt<TypeKind::UNKNOWN>(
-    ::duckdb::DataChunk* dataChunk,
-    int32_t row,
-    int32_t column) {
-  return variant::null(TypeKind::UNKNOWN);
-}
-
 template <TypeKind kind>
 variant variantAt(const ::duckdb::Value& value) {
   if (value.type() == ::duckdb::LogicalType::INTERVAL) {
@@ -296,11 +288,6 @@ variant variantAt(const ::duckdb::Value& value) {
   }
 }
 
-template <>
-variant variantAt<TypeKind::UNKNOWN>(const ::duckdb::Value& value) {
-  return variant::null(TypeKind::UNKNOWN);
-}
-
 template <>
 velox::variant variantAt<TypeKind::HUGEINT>(const ::duckdb::Value& value) {
   auto hugeInt = ::duckdb::HugeIntValue::Get(value);

diff --git a/velox/expression/CastExpr.cpp b/velox/expression/CastExpr.cpp
@@ -447,16 +447,6 @@ VectorPtr CastExpr::applyArray(
   return result;
 }
 
-// Cast from unknown type to other types.
-VectorPtr CastExpr::applyUnknown(
-    const SelectivityVector& rows,
-    const BaseVector& /*input*/,
-    exec::EvalCtx& context,
-    const TypePtr& /*fromType*/,
-    const TypePtr& toType) {
-  return BaseVector::createNullConstant(toType, rows.end(), context.pool());
-}
-
 VectorPtr CastExpr::applyRow(
     const SelectivityVector& rows,
     const RowVector* input,
@@ -735,7 +725,8 @@ void CastExpr::applyPeeled(
             toType);
         break;
       case TypeKind::UNKNOWN:
-        result = applyUnknown(rows, input, context, fromType, toType);
+        result =
+            BaseVector::createNullConstant(toType, rows.end(), context.pool());
         break;
       default: {
         // Handle primitive type conversions.

diff --git a/velox/expression/CastExpr.h b/velox/expression/CastExpr.h
@@ -113,13 +113,6 @@ class CastExpr : public SpecialForm {
       const TypePtr& toType,
       VectorPtr& result);
 
-  VectorPtr applyUnknown(
-      const SelectivityVector& rows,
-      const BaseVector& /*input*/,
-      exec::EvalCtx& context,
-      const TypePtr& /*fromType*/,
-      const TypePtr& toType);
-
   VectorPtr applyMap(
       const SelectivityVector& rows,
       const MapVector* input,

diff --git a/velox/expression/tests/CastExprTest.cpp b/velox/expression/tests/CastExprTest.cpp
@@ -372,8 +372,14 @@ TEST_F(CastExprTest, basics) {
 }
 
 TEST_F(CastExprTest, fromUnknownType) {
-  testCast<UnknownValue, int>(
+  testCast<UnknownValue, int8_t>(
+      "tinyint", {std::nullopt, std::nullopt}, {std::nullopt, std::nullopt});
+  testCast<UnknownValue, int16_t>(
+      "smallint", {std::nullopt, std::nullopt}, {std::nullopt, std::nullopt});
+  testCast<UnknownValue, int32_t>(
       "int", {std::nullopt, std::nullopt}, {std::nullopt, std::nullopt});
+  testCast<UnknownValue, int64_t>(
+      "bigint", {std::nullopt, std::nullopt}, {std::nullopt, std::nullopt});
   testCast<UnknownValue, float>(
       "float", {std::nullopt, std::nullopt}, {std::nullopt, std::nullopt});
   testCast<UnknownValue, double>(
@@ -382,6 +388,14 @@ TEST_F(CastExprTest, fromUnknownType) {
       "string", {std::nullopt, std::nullopt}, {std::nullopt, std::nullopt});
   testCast<UnknownValue, bool>(
       "boolean", {std::nullopt, std::nullopt}, {std::nullopt, std::nullopt});
+  testCast<UnknownValue, Timestamp>(
+      "timestamp", {std::nullopt, std::nullopt}, {std::nullopt, std::nullopt});
+  testCast<UnknownValue, int32_t>(
+      "date",
+      {std::nullopt, std::nullopt},
+      {std::nullopt, std::nullopt},
+      UNKNOWN(),
+      DATE());
 }
 
 TEST_F(CastExprTest, realAndDoubleToString) {

diff --git a/velox/functions/prestosql/aggregates/PrestoHasher.cpp b/velox/functions/prestosql/aggregates/PrestoHasher.cpp
@@ -116,13 +116,6 @@ FOLLY_ALWAYS_INLINE void PrestoHasher::hash(
   hashIntegral<T>(*vector_.get(), rows, hashes);
 }
 
-template <>
-FOLLY_ALWAYS_INLINE void PrestoHasher::hash<TypeKind::UNKNOWN>(
-    const SelectivityVector& rows,
-    BufferPtr& hashes) {
-  applyHashFunction(rows, *vector_.get(), hashes, [&](auto row) { return 0; });
-}
-
 template <>
 FOLLY_ALWAYS_INLINE void PrestoHasher::hash<TypeKind::BOOLEAN>(
     const SelectivityVector& rows,

diff --git a/velox/functions/prestosql/types/JsonType.cpp b/velox/functions/prestosql/types/JsonType.cpp
@@ -586,21 +586,14 @@ simdjson::error_code appendMapKey(
     const std::string_view& value,
     exec::GenericWriter& writer) {
   using T = typename TypeTraits<kind>::NativeType;
-  if constexpr (std::is_same_v<T, void> || std::is_same_v<T, UnknownValue>) {
+  if constexpr (std::is_same_v<T, void>) {
     return simdjson::INCORRECT_TYPE;
   } else {
     SIMDJSON_ASSIGN_OR_RAISE(writer.castTo<T>(), fromString<T>(value));
     return simdjson::SUCCESS;
   }
 }
 
-template <>
-simdjson::error_code appendMapKey<TypeKind::UNKNOWN>(
-    const std::string_view& value,
-    exec::GenericWriter& writer) {
-  VELOX_NYI("UNKNOWN type is not supported!");
-}
-
 template <>
 simdjson::error_code appendMapKey<TypeKind::VARCHAR>(
     const std::string_view& value,

diff --git a/velox/type/Type.h b/velox/type/Type.h
@@ -129,10 +129,6 @@ struct UnknownValue {
   bool operator>=(const UnknownValue& /* b */) const {
     return true;
   }
-
-  operator std::string() const {
-    return "NULL";
-  }
 };
 
 template <typename T>
@@ -1424,10 +1420,6 @@ std::shared_ptr<const OpaqueType> OPAQUE() {
         return TEMPLATE_FUNC<::facebook::velox::TypeKind::TIMESTAMP>(         \
             __VA_ARGS__);                                                     \
       }                                                                       \
-      case ::facebook::velox::TypeKind::UNKNOWN: {                            \
-        return TEMPLATE_FUNC<::facebook::velox::TypeKind::UNKNOWN>(           \
-            __VA_ARGS__);                                                     \
-      }                                                                       \
       default:                                                                \
         VELOX_FAIL(                                                           \
             "not a scalar type! kind: {}", mapTypeKindToName(typeKind));      \
@@ -1560,15 +1552,8 @@ std::shared_ptr<const OpaqueType> OPAQUE() {
     }                                                                          \
   }()
 
-#define VELOX_DYNAMIC_TYPE_DISPATCH(TEMPLATE_FUNC, typeKind, ...)              \
-  [&]() {                                                                      \
-    if ((typeKind) == ::facebook::velox::TypeKind::UNKNOWN) {                  \
-      return TEMPLATE_FUNC<::facebook::velox::TypeKind::UNKNOWN>(__VA_ARGS__); \
-    } else {                                                                   \
-      return VELOX_DYNAMIC_TYPE_DISPATCH_IMPL(                                 \
-          TEMPLATE_FUNC, , typeKind, __VA_ARGS__);                             \
-    }                                                                          \
-  }()
+#define VELOX_DYNAMIC_TYPE_DISPATCH(TEMPLATE_FUNC, typeKind, ...) \
+  VELOX_DYNAMIC_TYPE_DISPATCH_IMPL(TEMPLATE_FUNC, , typeKind, __VA_ARGS__)
 
 #define VELOX_DYNAMIC_TYPE_DISPATCH_ALL(TEMPLATE_FUNC, typeKind, ...)          \
   [&]() {                                                                      \