From 240223fab10db027484f8d3b179dd398ec63d084 Mon Sep 17 00:00:00 2001 From: PHILO-HE Date: Fri, 16 Feb 2024 22:33:34 +0800 Subject: [PATCH] Fix comments --- velox/exec/tests/utils/QueryAssertions.cpp | 13 ------------- velox/expression/CastExpr.cpp | 13 ++----------- velox/expression/CastExpr.h | 7 ------- velox/expression/tests/CastExprTest.cpp | 16 +++++++++++++++- .../prestosql/aggregates/PrestoHasher.cpp | 7 ------- velox/functions/prestosql/types/JsonType.cpp | 9 +-------- velox/type/Type.h | 19 ++----------------- 7 files changed, 20 insertions(+), 64 deletions(-) diff --git a/velox/exec/tests/utils/QueryAssertions.cpp b/velox/exec/tests/utils/QueryAssertions.cpp index 48340385c5ea2..0a4ebdf3ccfd9 100644 --- a/velox/exec/tests/utils/QueryAssertions.cpp +++ b/velox/exec/tests/utils/QueryAssertions.cpp @@ -274,14 +274,6 @@ variant variantAt( dataChunk->GetValue(column, row).GetValue<::duckdb::timestamp_t>())); } -template <> -variant variantAt( - ::duckdb::DataChunk* dataChunk, - int32_t row, - int32_t column) { - return variant::null(TypeKind::UNKNOWN); -} - template 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(const ::duckdb::Value& value) { - return variant::null(TypeKind::UNKNOWN); -} - template <> velox::variant variantAt(const ::duckdb::Value& value) { auto hugeInt = ::duckdb::HugeIntValue::Get(value); diff --git a/velox/expression/CastExpr.cpp b/velox/expression/CastExpr.cpp index dfa7f0a224d1b..d5b2554f25e76 100644 --- 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 index eb0000c7c856b..6a7e57428b11e 100644 --- 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 index 41201eabc4656..bb95e485fe5cd 100644 --- 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( + testCast( + "tinyint", {std::nullopt, std::nullopt}, {std::nullopt, std::nullopt}); + testCast( + "smallint", {std::nullopt, std::nullopt}, {std::nullopt, std::nullopt}); + testCast( "int", {std::nullopt, std::nullopt}, {std::nullopt, std::nullopt}); + testCast( + "bigint", {std::nullopt, std::nullopt}, {std::nullopt, std::nullopt}); testCast( "float", {std::nullopt, std::nullopt}, {std::nullopt, std::nullopt}); testCast( @@ -382,6 +388,14 @@ TEST_F(CastExprTest, fromUnknownType) { "string", {std::nullopt, std::nullopt}, {std::nullopt, std::nullopt}); testCast( "boolean", {std::nullopt, std::nullopt}, {std::nullopt, std::nullopt}); + testCast( + "timestamp", {std::nullopt, std::nullopt}, {std::nullopt, std::nullopt}); + testCast( + "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 index dd40cbc871768..75cf883768b6e 100644 --- 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(*vector_.get(), rows, hashes); } -template <> -FOLLY_ALWAYS_INLINE void PrestoHasher::hash( - const SelectivityVector& rows, - BufferPtr& hashes) { - applyHashFunction(rows, *vector_.get(), hashes, [&](auto row) { return 0; }); -} - template <> FOLLY_ALWAYS_INLINE void PrestoHasher::hash( const SelectivityVector& rows, diff --git a/velox/functions/prestosql/types/JsonType.cpp b/velox/functions/prestosql/types/JsonType.cpp index f159c99538cac..9defbf9c7c98a 100644 --- a/velox/functions/prestosql/types/JsonType.cpp +++ b/velox/functions/prestosql/types/JsonType.cpp @@ -586,7 +586,7 @@ simdjson::error_code appendMapKey( const std::string_view& value, exec::GenericWriter& writer) { using T = typename TypeTraits::NativeType; - if constexpr (std::is_same_v || std::is_same_v) { + if constexpr (std::is_same_v) { return simdjson::INCORRECT_TYPE; } else { SIMDJSON_ASSIGN_OR_RAISE(writer.castTo(), fromString(value)); @@ -594,13 +594,6 @@ simdjson::error_code appendMapKey( } } -template <> -simdjson::error_code appendMapKey( - const std::string_view& value, - exec::GenericWriter& writer) { - VELOX_NYI("UNKNOWN type is not supported!"); -} - template <> simdjson::error_code appendMapKey( const std::string_view& value, diff --git a/velox/type/Type.h b/velox/type/Type.h index 3e407df574c75..b2dfd75954f33 100644 --- 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 @@ -1424,10 +1420,6 @@ std::shared_ptr 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 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, ...) \ [&]() { \