[GitHub] [incubator-doris] Gabriel39 commented on a diff in pull request #9582: [refactor](decimalv3) optimize decimal performance and precision

[GitBox <gi...@apache.org>]

Gabriel39 commented on code in PR #9582:
URL: https://github.com/apache/incubator-doris/pull/9582#discussion_r873280037


##########
be/src/common/config.h:
##########
@@ -736,6 +736,12 @@ CONF_Validator(string_type_length_soft_limit_bytes,
 // used for olap scanner to save memory, when the size of unused_object_pool
 // is greater than object_pool_buffer_size, release the object in the unused_object_pool.
 CONF_Int32(object_pool_buffer_size, "100");
+
+// decimalv3: Decimal32/Decimal64/Decimal128
+// decimalv2 is converted to decimalv3 for calculation when enable_engine_decimalv3 is true.
+CONF_Bool(enable_engine_decimalv3, "false");

Review Comment:
   ```suggestion
   CONF_Bool(enable_execution_decimalv3, "false");
   ```



##########
be/src/vec/aggregate_functions/aggregate_function_avg.h:
##########
@@ -46,13 +46,15 @@ struct AggregateFunctionAvgData {
             // null is handled in AggregationNode::_get_without_key_result
             return static_cast<ResultT>(sum);
         }
-        // to keep the same result with row vesion; see AggregateFunctions::decimalv2_avg_get_value
-        if constexpr (std::is_same_v<ResultT, Decimal128> && std::is_same_v<T, Decimal128>) {
-            DecimalV2Value decimal_val_count(count, 0);
-            DecimalV2Value decimal_val_sum(static_cast<Int128>(sum));
-            DecimalV2Value cal_ret = decimal_val_sum / decimal_val_count;
-            Decimal128 ret(cal_ret.value());
-            return ret;
+        if (!config::enable_engine_decimalv3) {

Review Comment:
   ```suggestion
           if (!config::enable_execution_decimalv3) {
   ```



##########
be/src/olap/types.h:
##########
@@ -878,6 +893,87 @@ struct FieldTypeTraits<OLAP_FIELD_TYPE_DECIMAL>
     }
 };
 
+template <>
+struct FieldTypeTraits<OLAP_FIELD_TYPE_DECIMAL32>
+        : public BaseFieldtypeTraits<OLAP_FIELD_TYPE_DECIMAL32> {
+    static void set_to_max(void* buf) {
+        CppType* data = reinterpret_cast<CppType*>(buf);
+        *data = 999999999;
+    }
+    static void set_to_min(void* buf) {
+        CppType* data = reinterpret_cast<CppType*>(buf);
+        *data = -999999999;
+    }
+};
+
+template <>
+struct FieldTypeTraits<OLAP_FIELD_TYPE_DECIMAL64>
+        : public BaseFieldtypeTraits<OLAP_FIELD_TYPE_DECIMAL64> {
+    static void set_to_max(void* buf) {
+        CppType* data = reinterpret_cast<CppType*>(buf);
+        *data = 999999999999999999ll;
+    }
+    static void set_to_min(void* buf) {
+        CppType* data = reinterpret_cast<CppType*>(buf);
+        *data = -999999999999999999ll;
+    }
+};
+
+template <>
+struct FieldTypeTraits<OLAP_FIELD_TYPE_DECIMAL128>
+        : public BaseFieldtypeTraits<OLAP_FIELD_TYPE_DECIMAL128> {
+    static Status from_string(void* buf, const std::string& scan_key) {
+        int128_t value = 0;
+        const char* value_string = scan_key.c_str();
+        char* end = nullptr;
+        value = strtol(value_string, &end, 10);
+        if (*end != 0) {
+            value = 0;
+        } else if (value > LONG_MIN && value < LONG_MAX) {
+            // use strtol result directly
+        } else {
+            bool is_negative = false;
+            if (*value_string == '-' || *value_string == '+') {
+                if (*(value_string++) == '-') {
+                    is_negative = true;
+                }
+            }
+            uint128_t current = 0;
+            uint128_t max_int128 = ~((int128_t)(1) << 127);
+            while (*value_string != 0) {
+                if (current > max_int128 / 10) {
+                    break;
+                }
+                current = current * 10 + (*(value_string++) - '0');
+            }
+            if (*value_string != 0 || (!is_negative && current > max_int128) ||
+                (is_negative && current > max_int128 + 1)) {
+                current = 0;
+            }
+            value = is_negative ? -current : current;
+        }
+
+        *reinterpret_cast<PackedInt128*>(buf) = value;
+        return Status::OK();
+    }
+    static std::string to_string(const void* src) {
+        int128_t value = reinterpret_cast<const PackedInt128*>(src)->value;
+        fmt::memory_buffer buffer;
+        fmt::format_to(buffer, "{}", value);
+        return std::string(buffer.data(), buffer.size());
+    }
+    static void set_to_max(void* buf) {
+        *reinterpret_cast<PackedInt128*>(buf) =
+                static_cast<int128_t>(999999999999999999ll) * 100000000000000000ll * 1000ll +

Review Comment:
   Better to use hexadecimal numbers



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

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