You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by GitBox <gi...@apache.org> on 2020/11/24 03:45:47 UTC

[GitHub] [incubator-doris] chaoyli commented on a change in pull request #4938: [Schema change] Support More column type in schema change

chaoyli commented on a change in pull request #4938:
URL: https://github.com/apache/incubator-doris/pull/4938#discussion_r529182465



##########
File path: be/src/olap/types.h
##########
@@ -509,98 +530,55 @@ OLAPStatus convert_int_from_varchar(void* dest, const void* src) {
 }
 
 template <typename T>
-OLAPStatus convert_float_from_varchar(void* dest, const void* src) {
-    using SrcType = typename CppTypeTraits<OLAP_FIELD_TYPE_VARCHAR>::CppType;
-    auto src_value = reinterpret_cast<const SrcType *>(src);
-    StringParser::ParseResult parse_res;
-    T result = StringParser::string_to_float<T>(src_value->get_data(), src_value->get_size(), &parse_res);
-    if (UNLIKELY(parse_res != StringParser::PARSE_SUCCESS)) {
-        return OLAPStatus::OLAP_ERR_INVALID_SCHEMA;
-    }
-    *reinterpret_cast<T*>(dest) = result;
-    return OLAPStatus::OLAP_SUCCESS;
+OLAPStatus arithmetic_convert_from_char(void* dest, const void* src) {
+    prepare_char_before_convert(src);
+    return arithmetic_convert_from_varchar<T>(dest, src);
 }
 
-template<FieldType field_type>
-struct FieldTypeTraits : public BaseFieldtypeTraits<field_type> { };
+// Using ArithmeTicFieldtypeTraits to Derived code for OLAP_FIELD_TYPE_XXXINT, OLAP_FIELD_TYPE_FLOAT,
+// OLAP_FIELD_TYPE_DOUBLE, to reduce redundant code
+template <FieldType fieldType, bool isArithmetic>
+struct ArithmeTicFieldtypeTraits : public BaseFieldtypeTraits<fieldType> {
+    using CppType = typename CppTypeTraits<fieldType>::CppType;
 
-template<>
-struct FieldTypeTraits<OLAP_FIELD_TYPE_BOOL> : public BaseFieldtypeTraits<OLAP_FIELD_TYPE_BOOL> {
     static std::string to_string(const void* src) {
-        char buf[1024] = {'\0'};
-        snprintf(buf, sizeof(buf), "%d", *reinterpret_cast<const bool*>(src));
-        return std::string(buf);
+        return std::to_string(*reinterpret_cast<const CppType*>(src));
     }
-    static void set_to_max(void* buf) {
-        (*(bool*)buf) = true;
-    }
-    static void set_to_min(void* buf) {
-        (*(bool*)buf) = false;
-    }
-};
 
-template<>
-struct FieldTypeTraits<OLAP_FIELD_TYPE_TINYINT> : public BaseFieldtypeTraits<OLAP_FIELD_TYPE_TINYINT> {
-    static std::string to_string(const void* src) {
-        char buf[1024] = {'\0'};
-        snprintf(buf, sizeof(buf), "%d", *reinterpret_cast<const int8_t*>(src));
-        return std::string(buf);
-    }
     static OLAPStatus convert_from(void* dest, const void* src, const TypeInfo* src_type, MemPool* mem_pool) {
         if (src_type->type() == OLAP_FIELD_TYPE_VARCHAR) {
-            return convert_int_from_varchar<CppType>(dest, src);
+            return arithmetic_convert_from_varchar<CppType>(dest, src);
+        } else if (src_type->type() == OLAP_FIELD_TYPE_CHAR) {
+            return arithmetic_convert_from_char<CppType>(dest, src);
         }
         return OLAPStatus::OLAP_ERR_INVALID_SCHEMA;
     }
 };
 
-template<>
-struct FieldTypeTraits<OLAP_FIELD_TYPE_SMALLINT> : public BaseFieldtypeTraits<OLAP_FIELD_TYPE_SMALLINT> {
-    static std::string to_string(const void* src) {
-        char buf[1024] = {'\0'};
-        snprintf(buf, sizeof(buf), "%d", *reinterpret_cast<const int16_t*>(src));
-        return std::string(buf);
-    }
-    static OLAPStatus convert_from(void* dest, const void* src, const TypeInfo* src_type, MemPool* mem_pool) {
-        if (src_type->type() == OLAP_FIELD_TYPE_VARCHAR) {
-            return convert_int_from_varchar<CppType>(dest, src);
-        }
-        return OLAPStatus::OLAP_ERR_INVALID_SCHEMA;
-    }
-};
+template <FieldType fieldType>
+struct ArithmeTicFieldtypeTraits<fieldType, false> : public BaseFieldtypeTraits<fieldType> {};
 
-template<>
-struct FieldTypeTraits<OLAP_FIELD_TYPE_INT> : public BaseFieldtypeTraits<OLAP_FIELD_TYPE_INT> {
-    static std::string to_string(const void* src) {
-        char buf[1024] = {'\0'};
-        snprintf(buf, sizeof(buf), "%d", *reinterpret_cast<const int32_t *>(src));
-        return std::string(buf);
-    }
-    static OLAPStatus convert_from(void* dest, const void* src, const TypeInfo* src_type, MemPool* mem_pool) {
-        if (src_type->type() == OLAP_FIELD_TYPE_VARCHAR) {
-            return convert_int_from_varchar<CppType>(dest, src);
-        }
-        return OLAPStatus::OLAP_ERR_INVALID_SCHEMA;
-    }
-};
+template <FieldType fieldType>
+struct FieldTypeTraits : public ArithmeTicFieldtypeTraits<fieldType,
+        std::is_arithmetic<typename BaseFieldtypeTraits<fieldType>::CppType>::value && std::is_signed<typename BaseFieldtypeTraits<fieldType>::CppType>::value> {};

Review comment:
       use std::is_signed for what?

##########
File path: be/src/olap/types.h
##########
@@ -495,12 +496,32 @@ struct BaseFieldtypeTraits : public CppTypeTraits<field_type> {
     }
 };
 
+static void prepare_char_before_convert(const void* src) {
+    Slice* slice = const_cast<Slice*>(reinterpret_cast<const Slice*>(src));
+    char* buf = slice->data;
+    auto p = slice->size - 1;
+    while (p >= 0 && buf[p] == '\0') {
+        p--;
+    }
+    slice->size = p + 1;
+}
+
 template <typename T>
-OLAPStatus convert_int_from_varchar(void* dest, const void* src) {
-    using SrcType = typename CppTypeTraits<OLAP_FIELD_TYPE_VARCHAR>::CppType;
-    auto src_value = reinterpret_cast<const SrcType*>(src);
+T convert_from_varchar(const Slice* src_value, StringParser::ParseResult& parse_res, std::true_type) {
+    return StringParser::string_to_int<T>(src_value->get_data(), src_value->get_size(), &parse_res);
+}
+
+template <typename T>

Review comment:
       There may use traits ?
   Or use if constexpr (std::is_floating_point_v<T>)) to distinguish the two cases.

##########
File path: be/src/olap/types.h
##########
@@ -509,98 +530,55 @@ OLAPStatus convert_int_from_varchar(void* dest, const void* src) {
 }
 
 template <typename T>
-OLAPStatus convert_float_from_varchar(void* dest, const void* src) {
-    using SrcType = typename CppTypeTraits<OLAP_FIELD_TYPE_VARCHAR>::CppType;
-    auto src_value = reinterpret_cast<const SrcType *>(src);
-    StringParser::ParseResult parse_res;
-    T result = StringParser::string_to_float<T>(src_value->get_data(), src_value->get_size(), &parse_res);
-    if (UNLIKELY(parse_res != StringParser::PARSE_SUCCESS)) {
-        return OLAPStatus::OLAP_ERR_INVALID_SCHEMA;
-    }
-    *reinterpret_cast<T*>(dest) = result;
-    return OLAPStatus::OLAP_SUCCESS;
+OLAPStatus arithmetic_convert_from_char(void* dest, const void* src) {
+    prepare_char_before_convert(src);
+    return arithmetic_convert_from_varchar<T>(dest, src);
 }
 
-template<FieldType field_type>
-struct FieldTypeTraits : public BaseFieldtypeTraits<field_type> { };
+// Using ArithmeTicFieldtypeTraits to Derived code for OLAP_FIELD_TYPE_XXXINT, OLAP_FIELD_TYPE_FLOAT,
+// OLAP_FIELD_TYPE_DOUBLE, to reduce redundant code
+template <FieldType fieldType, bool isArithmetic>
+struct ArithmeTicFieldtypeTraits : public BaseFieldtypeTraits<fieldType> {

Review comment:
       Numeric may be a better name.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org