Merge pull request #22003 from ClickHouse/aku/field-reinterpret

prevent accidental reinterpret_cast in Field::get<>

src/AggregateFunctions/AggregateFunctionSumMap.h

@@ -413,7 +413,7 @@ public:
 
         for (const Field & f : keys_to_keep_)
         {
-            keys_to_keep.emplace(f.safeGet<NearestFieldType<T>>());
+            keys_to_keep.emplace(f.safeGet<T>());
         }
     }
 

src/Columns/ColumnConst.h

@@ -255,7 +255,7 @@ public:
 
     /// The constant value. It is valid even if the size of the column is 0.
     template <typename T>
-    T getValue() const { return getField().safeGet<NearestFieldType<T>>(); }
+    T getValue() const { return getField().safeGet<T>(); }
 
     bool isCollationSupported() const override { return data->isCollationSupported(); }
 };

src/Columns/ColumnDecimal.h

@@ -107,7 +107,7 @@ public:
     {
         data.resize_fill(data.size() + length);
     }
-    void insert(const Field & x) override { data.push_back(DB::get<NearestFieldType<T>>(x)); }
+    void insert(const Field & x) override { data.push_back(DB::get<T>(x)); }
     void insertRangeFrom(const IColumn & src, size_t start, size_t length) override;
 
     void popBack(size_t n) override

src/Columns/ColumnVector.h

@@ -261,7 +261,7 @@ public:
 
     void insert(const Field & x) override
     {
-        data.push_back(DB::get<NearestFieldType<T>>(x));
+        data.push_back(DB::get<T>(x));
     }
 
     void insertRangeFrom(const IColumn & src, size_t start, size_t length) override;

src/Core/Field.h

@@ -399,10 +399,10 @@ public:
 
 
     template <typename T>
-    T & get();
+    NearestFieldType<std::decay_t<T>> & get();
 
     template <typename T>
-    const T & get() const
+    const auto & get() const
     {
         auto mutable_this = const_cast<std::decay_t<decltype(*this)> *>(this);
         return mutable_this->get<T>();
@@ -436,21 +436,10 @@ public:
         return true;
     }
 
-    template <typename T> T & safeGet()
-    {
-        const Types::Which requested = TypeToEnum<std::decay_t<T>>::value;
-        if (which != requested)
-            throw Exception("Bad get: has " + std::string(getTypeName()) + ", requested " + std::string(Types::toString(requested)), ErrorCodes::BAD_GET);
-        return get<T>();
-    }
+    template <typename T> auto & safeGet() const
+    { return const_cast<Field *>(this)->safeGet<T>(); }
 
-    template <typename T> const T & safeGet() const
-    {
-        const Types::Which requested = TypeToEnum<std::decay_t<T>>::value;
-        if (which != requested)
-            throw Exception("Bad get: has " + std::string(getTypeName()) + ", requested " + std::string(Types::toString(requested)), ErrorCodes::BAD_GET);
-        return get<T>();
-    }
+    template <typename T> auto & safeGet();
 
     bool operator< (const Field & rhs) const
     {
@@ -778,22 +767,40 @@ inline constexpr bool isInt64FieldType(Field::Types::Which t)
 
 // Field value getter with type checking in debug builds.
 template <typename T>
-T & Field::get()
+NearestFieldType<std::decay_t<T>> & Field::get()
 {
-    using ValueType = std::decay_t<T>;
+    // Before storing the value in the Field, we static_cast it to the field
+    // storage type, so here we return the value of storage type as well.
+    // Otherwise, it is easy to make a mistake of reinterpret_casting the stored
+    // value to a different and incompatible type.
+    // For example, a Float32 value is stored as Float64, and it is incorrect to
+    // return a reference to this value as Float32.
+    using StoredType = NearestFieldType<std::decay_t<T>>;
 
 #ifndef NDEBUG
     // Disregard signedness when converting between int64 types.
-    constexpr Field::Types::Which target = TypeToEnum<NearestFieldType<ValueType>>::value;
+    constexpr Field::Types::Which target = TypeToEnum<StoredType>::value;
     if (target != which
            && (!isInt64FieldType(target) || !isInt64FieldType(which)))
         throw Exception(ErrorCodes::LOGICAL_ERROR, "Invalid Field get from type {} to type {}", Types::toString(which), Types::toString(target));
 #endif
 
-    ValueType * MAY_ALIAS ptr = reinterpret_cast<ValueType *>(&storage);
+    StoredType * MAY_ALIAS ptr = reinterpret_cast<StoredType *>(&storage);
+
     return *ptr;
 }
 
+
+template <typename T>
+auto & Field::safeGet()
+{
+    const Types::Which requested = TypeToEnum<NearestFieldType<std::decay_t<T>>>::value;
+    if (which != requested)
+        throw Exception("Bad get: has " + std::string(getTypeName()) + ", requested " + std::string(Types::toString(requested)), ErrorCodes::BAD_GET);
+    return get<T>();
+}
+
+
 template <typename T>
 T & Field::reinterpret()
 {

src/DataTypes/DataTypeEnum.cpp

@@ -105,7 +105,7 @@ DataTypeEnum<Type>::DataTypeEnum(const Values & values_) : values{values_}
 template <typename Type>
 void DataTypeEnum<Type>::serializeBinary(const Field & field, WriteBuffer & ostr) const
 {
-    const FieldType x = get<NearestFieldType<FieldType>>(field);
+    const FieldType x = get<FieldType>(field);
     writeBinary(x, ostr);
 }
 
@@ -405,7 +405,7 @@ static DataTypePtr createExact(const ASTPtr & arguments)
                 ErrorCodes::UNEXPECTED_AST_STRUCTURE);
 
         const String & field_name = name_literal->value.get<String>();
-        const auto value = value_literal->value.get<NearestFieldType<FieldType>>();
+        const auto value = value_literal->value.get<FieldType>();
 
         if (value > std::numeric_limits<FieldType>::max() || value < std::numeric_limits<FieldType>::min())
             throw Exception{"Value " + toString(value) + " for element '" + field_name + "' exceeds range of " + EnumName<FieldType>::value,

src/DataTypes/DataTypeNumberBase.cpp

@@ -152,7 +152,7 @@ template <typename T>
 void DataTypeNumberBase<T>::serializeBinary(const Field & field, WriteBuffer & ostr) const
 {
     /// ColumnVector<T>::ValueType is a narrower type. For example, UInt8, when the Field type is UInt64
-    typename ColumnVector<T>::ValueType x = get<NearestFieldType<FieldType>>(field);
+    typename ColumnVector<T>::ValueType x = get<FieldType>(field);
     writeBinary(x, ostr);
 }
 

src/Dictionaries/ComplexKeyHashedDictionary.cpp

@@ -338,7 +338,7 @@ void ComplexKeyHashedDictionary::calculateBytesAllocated()
 template <typename T>
 void ComplexKeyHashedDictionary::createAttributeImpl(Attribute & attribute, const Field & null_value)
 {
-    attribute.null_values = T(null_value.get<NearestFieldType<T>>());
+    attribute.null_values = T(null_value.get<T>());
     attribute.maps.emplace<ContainerType<T>>();
 }
 
@@ -450,7 +450,7 @@ bool ComplexKeyHashedDictionary::setAttributeValue(Attribute & attribute, const
             }
         }
 
-        result = setAttributeValueImpl<AttributeType>(attribute, key, value.get<NearestFieldType<AttributeType>>());
+        result = setAttributeValueImpl<AttributeType>(attribute, key, value.get<AttributeType>());
     };
 
     callOnDictionaryAttributeType(attribute.type, type_call);

src/Dictionaries/FlatDictionary.cpp

@@ -370,7 +370,7 @@ void FlatDictionary::calculateBytesAllocated()
 template <typename T>
 void FlatDictionary::createAttributeImpl(Attribute & attribute, const Field & null_value)
 {
-    attribute.null_values = T(null_value.get<NearestFieldType<T>>());
+    attribute.null_values = T(null_value.get<T>());
     const auto & null_value_ref = std::get<T>(attribute.null_values);
     attribute.arrays.emplace<ContainerType<T>>(initial_array_size, null_value_ref);
 }
@@ -478,7 +478,7 @@ void FlatDictionary::setAttributeValue(Attribute & attribute, const Key id, cons
             }
         }
 
-        setAttributeValueImpl<AttributeType>(attribute, id, value.get<NearestFieldType<AttributeType>>());
+        setAttributeValueImpl<AttributeType>(attribute, id, value.get<AttributeType>());
     };
 
     callOnDictionaryAttributeType(attribute.type, type_call);

src/Dictionaries/HashedDictionary.cpp

@@ -451,7 +451,7 @@ void HashedDictionary::calculateBytesAllocated()
 template <typename T>
 void HashedDictionary::createAttributeImpl(Attribute & attribute, const Field & null_value)
 {
-    attribute.null_values = T(null_value.get<NearestFieldType<T>>());
+    attribute.null_values = T(null_value.get<T>());
     if (!sparse)
         attribute.maps = std::make_unique<CollectionType<T>>();
     else
@@ -565,7 +565,7 @@ bool HashedDictionary::setAttributeValue(Attribute & attribute, const Key id, co
             }
         }
 
-        result = setAttributeValueImpl<AttributeType>(attribute, id, value.get<NearestFieldType<AttributeType>>());
+        result = setAttributeValueImpl<AttributeType>(attribute, id, value.get<AttributeType>());
     };
 
     callOnDictionaryAttributeType(attribute.type, type_call);

src/Dictionaries/IPAddressDictionary.cpp

@@ -595,7 +595,7 @@ void IPAddressDictionary::calculateBytesAllocated()
 template <typename T>
 void IPAddressDictionary::createAttributeImpl(Attribute & attribute, const Field & null_value)
 {
-    attribute.null_values = null_value.isNull() ? T{} : T(null_value.get<NearestFieldType<T>>());
+    attribute.null_values = null_value.isNull() ? T{} : T(null_value.get<T>());
     attribute.maps.emplace<ContainerType<T>>();
 }
 
@@ -786,7 +786,7 @@ void IPAddressDictionary::setAttributeValue(Attribute & attribute, const Field &
         }
         else
         {
-            setAttributeValueImpl<AttributeType>(attribute, value.get<NearestFieldType<AttributeType>>());
+            setAttributeValueImpl<AttributeType>(attribute, value.get<AttributeType>());
         }
     };
 

src/Dictionaries/RangeHashedDictionary.cpp

@@ -350,7 +350,7 @@ void RangeHashedDictionary::calculateBytesAllocated()
 template <typename T>
 void RangeHashedDictionary::createAttributeImpl(Attribute & attribute, const Field & null_value)
 {
-    attribute.null_values = T(null_value.get<NearestFieldType<T>>());
+    attribute.null_values = T(null_value.get<T>());
     attribute.maps = std::make_unique<Collection<T>>();
 }
 
@@ -458,7 +458,7 @@ void RangeHashedDictionary::setAttributeValueImpl(Attribute & attribute, const K
         }
         else
         {
-            value_to_insert = Value<ValueType>{ range, { value.get<NearestFieldType<ValueType>>() }};
+            value_to_insert = Value<ValueType>{ range, { value.get<ValueType>() }};
         }
     }
 

src/Interpreters/ConvertStringsToEnumVisitor.h

@@ -41,8 +41,8 @@ String makeStringsEnum(const std::set<String> & values)
 
 void changeIfArguments(ASTPtr & first, ASTPtr & second)
 {
-    String first_value = first->as<ASTLiteral>()->value.get<NearestFieldType<String>>();
-    String second_value = second->as<ASTLiteral>()->value.get<NearestFieldType<String>>();
+    String first_value = first->as<ASTLiteral>()->value.get<String>();
+    String second_value = second->as<ASTLiteral>()->value.get<String>();
 
     std::set<String> values;
     values.insert(first_value);
@@ -67,9 +67,9 @@ void changeTransformArguments(ASTPtr & array_to, ASTPtr & other)
 {
     std::set<String> values;
 
-    for (const auto & item : array_to->as<ASTLiteral>()->value.get<NearestFieldType<Array>>())
-        values.insert(item.get<NearestFieldType<String>>());
-    values.insert(other->as<ASTLiteral>()->value.get<NearestFieldType<String>>());
+    for (const auto & item : array_to->as<ASTLiteral>()->value.get<Array>())
+        values.insert(item.get<String>());
+    values.insert(other->as<ASTLiteral>()->value.get<String>());
 
     String enum_string = makeStringsEnum(values);
 
@@ -197,7 +197,7 @@ struct ConvertStringsToEnumMatcher
                 String(literal_other->value.getTypeName()) != "String")
                 return;
 
-            Array array_to = literal_to->value.get<NearestFieldType<Array>>();
+            Array array_to = literal_to->value.get<Array>();
             if (array_to.size() == 0)
                 return;