Skip to content

Commit 240e508

Browse files
laithsakkafacebook-github-bot
authored andcommitted
remove CppToType usage from SimpleFunctionAdapter and ComplexViewTypes (#4420)
Summary: Pull Request resolved: #4420 Pull Request resolved: #4304 There is one remaining place, ComplexWriterTypes for the generic writer I will address that in a different diff since there are pending changes on that file that needs to land first. Reviewed By: kagamiori Differential Revision: D44388667 fbshipit-source-id: da2281dbdbcc95971d8ce3df100889deaebe9004
1 parent 7289b84 commit 240e508

File tree

7 files changed

+164
-52
lines changed

7 files changed

+164
-52
lines changed

velox/core/SimpleFunctionMetadata.h

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -191,14 +191,16 @@ struct TypeAnalysis {
191191
void run(TypeAnalysisResults& results) {
192192
// This should only handle primitives and OPAQUE.
193193
static_assert(
194-
CppToType<T>::isPrimitiveType ||
195-
CppToType<T>::typeKind == TypeKind::OPAQUE);
194+
SimpleTypeTrait<T>::isPrimitiveType ||
195+
SimpleTypeTrait<T>::typeKind == TypeKind::OPAQUE);
196196
results.stats.concreteCount++;
197-
if (isDecimalKind(CppToType<T>::typeKind)) {
198-
results.out << detail::strToLowerCopy(std::string(CppToType<T>::name))
197+
if (isDecimalKind(SimpleTypeTrait<T>::typeKind)) {
198+
results.out << detail::strToLowerCopy(
199+
std::string(SimpleTypeTrait<T>::name))
199200
<< "(" << kPrecisionVariable << "," << kScaleVariable << ")";
200201
} else {
201-
results.out << detail::strToLowerCopy(std::string(CppToType<T>::name));
202+
results.out << detail::strToLowerCopy(
203+
std::string(SimpleTypeTrait<T>::name));
202204
}
203205
}
204206
};

velox/expression/ComplexViewTypes.h

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1026,11 +1026,11 @@ class GenericView {
10261026
GenericView(
10271027
const DecodedVector& decoded,
10281028
std::array<std::shared_ptr<void>, 3>& castReaders,
1029-
TypePtr& castType,
1029+
std::optional<const std::type_info*>& castTypeInfo,
10301030
vector_size_t index)
10311031
: decoded_(decoded),
10321032
castReaders_(castReaders),
1033-
castType_(castType),
1033+
castTypeInfo_(castTypeInfo),
10341034
index_(index) {}
10351035

10361036
uint64_t hash() const {
@@ -1064,9 +1064,8 @@ class GenericView {
10641064
VELOX_DCHECK(
10651065
CastTypeChecker<ToType>::check(type()),
10661066
fmt::format(
1067-
"castTo type is not compatible with type of vector, vector type is {}, casted to type is {}",
1068-
type()->toString(),
1069-
CppToType<ToType>::create()->toString()));
1067+
"castTo type is not compatible with type of vector, vector type is {}",
1068+
type()->toString()));
10701069

10711070
// TODO: We can distinguish if this is a null-free or not null-free
10721071
// generic. And based on that determine if we want to call operator[] or
@@ -1103,17 +1102,18 @@ class GenericView {
11031102
// This is basically Array<Any>, Map<Any,Any>, Row<Any....>.
11041103
return ensureReaderImpl<B, 1>();
11051104
} else {
1106-
auto requestedType = CppToType<B>::create();
1107-
if (castType_) {
1105+
auto* requestedTypeId = &typeid(B);
1106+
if (castTypeInfo_.has_value()) {
11081107
VELOX_USER_CHECK(
1109-
castType_->operator==(*requestedType),
1108+
std::type_index(**castTypeInfo_) ==
1109+
std::type_index(*requestedTypeId),
11101110
fmt::format(
11111111
"Not allowed to cast to the two types {} and {} within the same batch."
11121112
"Consider creating a new type set to allow it.",
1113-
castType_->toString(),
1114-
requestedType->toString()));
1113+
typeid(B).name(),
1114+
castTypeInfo_.value()->name()));
11151115
} else {
1116-
castType_ = std::move(requestedType);
1116+
castTypeInfo_ = {requestedTypeId};
11171117
}
11181118
return ensureReaderImpl<B, 2>();
11191119
}
@@ -1133,7 +1133,7 @@ class GenericView {
11331133

11341134
const DecodedVector& decoded_;
11351135
std::array<std::shared_ptr<void>, 3>& castReaders_;
1136-
TypePtr& castType_;
1136+
std::optional<const std::type_info*>& castTypeInfo_;
11371137
vector_size_t index_;
11381138
};
11391139

velox/expression/GenericWriter.cpp

Lines changed: 40 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -22,14 +22,20 @@ namespace facebook::velox::exec {
2222

2323
namespace {
2424

25-
// Base case for primitives
26-
template <TypeKind T>
27-
void copy_from_internal(GenericWriter& out, const GenericView& in) {
28-
// Maybe we should print a warning asking the user to have a fast path
29-
// specialization for primitives and not to use this?.
30-
using native_t = typename TypeTraits<T>::NativeType;
31-
out.castTo<native_t>() = in.castTo<native_t>();
32-
}
25+
template <TypeKind kind>
26+
struct KindToSimpleType {
27+
using type = typename TypeTraits<kind>::NativeType;
28+
};
29+
30+
template <>
31+
struct KindToSimpleType<TypeKind::VARCHAR> {
32+
using type = Varchar;
33+
};
34+
35+
template <>
36+
struct KindToSimpleType<TypeKind::VARBINARY> {
37+
using type = Varbinary;
38+
};
3339

3440
// Fast path when array elements are primitives.
3541
template <TypeKind T>
@@ -39,11 +45,35 @@ void copy_from_internal_array_fast(GenericWriter& out, const GenericView& in) {
3945
VELOX_UNREACHABLE(
4046
"Element type for fast path of copy_from must be primitive.");
4147
} else {
42-
using native_t = typename TypeTraits<T>::NativeType;
43-
out.castTo<Array<native_t>>().copy_from(in.castTo<Array<native_t>>());
48+
using simple_element_t = typename KindToSimpleType<T>::type;
49+
out.castTo<Array<simple_element_t>>().copy_from(
50+
in.castTo<Array<simple_element_t>>());
4451
}
4552
}
4653

54+
// Base case for primitives.
55+
template <TypeKind T>
56+
void copy_from_internal(GenericWriter& out, const GenericView& in) {
57+
// Maybe we should print a warning asking the user to have a fast path
58+
// specialization for primitives and not to use this?.
59+
using simple_element_t = typename KindToSimpleType<T>::type;
60+
out.castTo<simple_element_t>() = in.castTo<simple_element_t>();
61+
}
62+
63+
template <>
64+
void copy_from_internal<TypeKind::VARBINARY>(
65+
GenericWriter& out,
66+
const GenericView& in) {
67+
out.castTo<Varbinary>().copy_from(in.castTo<Varbinary>());
68+
}
69+
70+
template <>
71+
void copy_from_internal<TypeKind::VARCHAR>(
72+
GenericWriter& out,
73+
const GenericView& in) {
74+
out.castTo<Varchar>().copy_from(in.castTo<Varchar>());
75+
}
76+
4777
template <>
4878
void copy_from_internal<TypeKind::ARRAY>(
4979
GenericWriter& out,

velox/expression/SimpleFunctionAdapter.h

Lines changed: 7 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -62,19 +62,10 @@ struct udf_reuse_strings_from_arg<
6262
util::detail::void_t<decltype(T::reuse_strings_from_arg)>>
6363
: std::integral_constant<int32_t, T::reuse_strings_from_arg> {};
6464

65-
struct GenericOutputTypeTrait {
66-
static constexpr TypeKind typeKind = TypeKind::INVALID;
67-
static constexpr bool isPrimitiveType = false;
68-
static constexpr bool isFixedWidth = false;
69-
};
70-
7165
template <typename FUNC>
7266
class SimpleFunctionAdapter : public VectorFunction {
7367
using T = typename FUNC::exec_return_type;
74-
using return_type_traits = std::conditional_t<
75-
isGenericType<typename FUNC::return_type>::value,
76-
GenericOutputTypeTrait,
77-
CppToType<typename FUNC::return_type>>;
68+
using return_type_traits = SimpleTypeTrait<typename FUNC::return_type>;
7869

7970
template <int32_t POSITION>
8071
using exec_arg_at = typename std::
@@ -94,9 +85,8 @@ class SimpleFunctionAdapter : public VectorFunction {
9485
// boolean.
9586
template <int32_t POSITION>
9687
static constexpr bool isArgFlatConstantFastPathEligible =
97-
!isGenericType<arg_at<POSITION>>::value &&
98-
CppToType<arg_at<POSITION>>::isPrimitiveType &&
99-
CppToType<arg_at<POSITION>>::typeKind != TypeKind::BOOLEAN;
88+
SimpleTypeTrait<arg_at<POSITION>>::isPrimitiveType&&
89+
SimpleTypeTrait<arg_at<POSITION>>::typeKind != TypeKind::BOOLEAN;
10090

10191
constexpr int32_t reuseStringsFromArgValue() const {
10292
return udf_reuse_strings_from_arg<typename FUNC::udf_struct_t>();
@@ -241,7 +231,8 @@ class SimpleFunctionAdapter : public VectorFunction {
241231
VectorPtr* findReusableArg(std::vector<VectorPtr>& args) const {
242232
if constexpr (isVariadicType<arg_at<POSITION>>::value) {
243233
if constexpr (
244-
CppToType<typename arg_at<POSITION>::underlying_type>::typeKind ==
234+
SimpleTypeTrait<
235+
typename arg_at<POSITION>::underlying_type>::typeKind ==
245236
return_type_traits::typeKind) {
246237
for (auto i = POSITION; i < args.size(); i++) {
247238
if (BaseVector::isVectorWritable(args[i])) {
@@ -253,7 +244,8 @@ class SimpleFunctionAdapter : public VectorFunction {
253244
// yet, we know for sure that we won't.
254245
return nullptr;
255246
} else if constexpr (
256-
CppToType<arg_at<POSITION>>::typeKind == return_type_traits::typeKind) {
247+
SimpleTypeTrait<arg_at<POSITION>>::typeKind ==
248+
return_type_traits::typeKind) {
257249
using type =
258250
typename VectorExec::template resolver<arg_at<POSITION>>::in_type;
259251
if (args[POSITION]->isFlatEncoding() && args[POSITION].unique() &&

velox/expression/VectorReaders.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@
2222
#include <optional>
2323
#include <string_view>
2424
#include <type_traits>
25+
#include <typeindex>
26+
#include <typeinfo>
2527
#include <utility>
2628

2729
#include "velox/expression/ComplexViewTypes.h"
@@ -697,7 +699,7 @@ struct VectorReader<Generic<T>> {
697699
// Those two variables are mutated by the GenericView during cast operations,
698700
// and are shared across GenericViews constructed by the reader.
699701
mutable std::array<std::shared_ptr<void>, 3> castReaders_;
700-
mutable TypePtr castType_ = nullptr;
702+
mutable std::optional<const std::type_info*> castType_ = std::nullopt;
701703
};
702704

703705
template <typename T>

velox/type/Type.h

Lines changed: 94 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1567,6 +1567,100 @@ struct CustomType {
15671567
CustomType() {}
15681568
};
15691569

1570+
struct Varbinary {
1571+
private:
1572+
Varbinary() {}
1573+
};
1574+
struct Varchar {
1575+
private:
1576+
Varchar() {}
1577+
};
1578+
1579+
template <typename T>
1580+
struct SimpleTypeTrait {};
1581+
1582+
template <>
1583+
struct SimpleTypeTrait<int64_t> : public TypeTraits<TypeKind::BIGINT> {};
1584+
1585+
template <>
1586+
struct SimpleTypeTrait<int32_t> : public TypeTraits<TypeKind::INTEGER> {};
1587+
1588+
template <>
1589+
struct SimpleTypeTrait<int16_t> : public TypeTraits<TypeKind::SMALLINT> {};
1590+
1591+
template <>
1592+
struct SimpleTypeTrait<int8_t> : public TypeTraits<TypeKind::TINYINT> {};
1593+
1594+
template <>
1595+
struct SimpleTypeTrait<float> : public TypeTraits<TypeKind::REAL> {};
1596+
1597+
template <>
1598+
struct SimpleTypeTrait<double> : public TypeTraits<TypeKind::DOUBLE> {};
1599+
1600+
template <>
1601+
struct SimpleTypeTrait<bool> : public TypeTraits<TypeKind::BOOLEAN> {};
1602+
1603+
template <>
1604+
struct SimpleTypeTrait<Varchar> : public TypeTraits<TypeKind::VARCHAR> {};
1605+
1606+
template <>
1607+
struct SimpleTypeTrait<Varbinary> : public TypeTraits<TypeKind::VARBINARY> {};
1608+
1609+
template <>
1610+
struct SimpleTypeTrait<Timestamp> : public TypeTraits<TypeKind::TIMESTAMP> {};
1611+
1612+
template <>
1613+
struct SimpleTypeTrait<Date> : public TypeTraits<TypeKind::DATE> {};
1614+
1615+
template <>
1616+
struct SimpleTypeTrait<IntervalDayTime>
1617+
: public TypeTraits<TypeKind::INTERVAL_DAY_TIME> {};
1618+
1619+
template <typename T>
1620+
struct SimpleTypeTrait<Generic<T>> {
1621+
static constexpr TypeKind typeKind = TypeKind::UNKNOWN;
1622+
static constexpr bool isPrimitiveType = false;
1623+
static constexpr bool isFixedWidth = false;
1624+
};
1625+
1626+
template <typename T>
1627+
struct SimpleTypeTrait<std::shared_ptr<T>>
1628+
: public TypeTraits<TypeKind::OPAQUE> {};
1629+
1630+
template <typename KEY, typename VAL>
1631+
struct SimpleTypeTrait<Map<KEY, VAL>> : public TypeTraits<TypeKind::MAP> {};
1632+
1633+
template <typename ELEMENT>
1634+
struct SimpleTypeTrait<Array<ELEMENT>> : public TypeTraits<TypeKind::ARRAY> {};
1635+
1636+
template <typename... T>
1637+
struct SimpleTypeTrait<Row<T...>> : public TypeTraits<TypeKind::ROW> {};
1638+
1639+
template <>
1640+
struct SimpleTypeTrait<DynamicRow> : public TypeTraits<TypeKind::ROW> {};
1641+
1642+
template <>
1643+
struct SimpleTypeTrait<UnscaledShortDecimal>
1644+
: public TypeTraits<TypeKind::SHORT_DECIMAL> {};
1645+
1646+
template <>
1647+
struct SimpleTypeTrait<UnscaledLongDecimal>
1648+
: public TypeTraits<TypeKind::LONG_DECIMAL> {};
1649+
1650+
// T is also a simple type that represent the physical type of the custom type.
1651+
template <typename T>
1652+
struct SimpleTypeTrait<CustomType<T>>
1653+
: public SimpleTypeTrait<typename T::type> {
1654+
using physical_t = SimpleTypeTrait<typename T::type>;
1655+
static constexpr TypeKind typeKind = physical_t::typeKind;
1656+
static constexpr bool isPrimitiveType = physical_t::isPrimitiveType;
1657+
static constexpr bool isFixedWidth = physical_t::isFixedWidth;
1658+
1659+
// This is different than the physical type name.
1660+
static constexpr char* name = T::typeName;
1661+
};
1662+
1663+
// TODO: move cppToType testing utilities.
15701664
template <typename T>
15711665
struct CppToType {};
15721666

@@ -1577,14 +1671,6 @@ struct CppToTypeBase : public TypeTraits<KIND> {
15771671
}
15781672
};
15791673

1580-
struct Varbinary {
1581-
private:
1582-
Varbinary() {}
1583-
};
1584-
struct Varchar {
1585-
private:
1586-
Varchar() {}
1587-
};
15881674
template <>
15891675
struct CppToType<int64_t> : public CppToTypeBase<TypeKind::BIGINT> {};
15901676

velox/vector/VectorTypeUtils.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ struct KindToFlatVector<TypeKind::OPAQUE> {
109109

110110
template <typename T>
111111
struct TypeToFlatVector {
112-
using type = typename KindToFlatVector<CppToType<T>::typeKind>::type;
112+
using type = typename KindToFlatVector<SimpleTypeTrait<T>::typeKind>::type;
113113
};
114114

115115
} // namespace velox

0 commit comments

Comments
 (0)