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:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to