feat(query): improve signed numeric keys

src/meta/proto-conv/tests/it/main.rs

@@ -33,3 +33,4 @@ mod v023_table_meta;
 mod v024_table_meta;
 mod v025_user_stage;
 mod v026_schema;
+mod v027_schema;

src/meta/proto-conv/tests/it/proto_conv.rs

@@ -16,6 +16,8 @@ use std::collections::BTreeMap;
 use std::collections::BTreeSet;
 use std::sync::Arc;
 
+use ce::types::decimal::DecimalSize;
+use ce::types::DecimalDataType;
 use ce::types::NumberDataType;
 use chrono::TimeZone;
 use chrono::Utc;
@@ -196,6 +198,37 @@ fn new_table_meta() -> mt::TableMeta {
     }
 }
 
+pub(crate) fn new_latest_schema() -> TableSchema {
+    let b1 = TableDataType::Tuple {
+        fields_name: vec!["b11".to_string(), "b12".to_string()],
+        fields_type: vec![TableDataType::Boolean, TableDataType::String],
+    };
+    let b = TableDataType::Tuple {
+        fields_name: vec!["b1".to_string(), "b2".to_string()],
+        fields_type: vec![b1, TableDataType::Number(NumberDataType::Int64)],
+    };
+    let fields = vec![
+        TableField::new("a", TableDataType::Number(NumberDataType::UInt64)),
+        TableField::new("b", b),
+        TableField::new("c", TableDataType::Number(NumberDataType::UInt64)),
+        TableField::new(
+            "decimal128",
+            TableDataType::Decimal(DecimalDataType::Decimal128(DecimalSize {
+                precision: 18,
+                scale: 3,
+            })),
+        ),
+        ce::TableField::new(
+            "decimal256",
+            TableDataType::Decimal(DecimalDataType::Decimal256(DecimalSize {
+                precision: 46,
+                scale: 6,
+            })),
+        ),
+    ];
+    TableSchema::new(fields)
+}
+
 pub(crate) fn new_table_copied_file_info_v6() -> mt::TableCopiedFileInfo {
     mt::TableCopiedFileInfo {
         etag: Some("etag".to_string()),
@@ -338,21 +371,7 @@ fn test_build_pb_buf() -> anyhow::Result<()> {
 
     // schema
     {
-        let b1 = TableDataType::Tuple {
-            fields_name: vec!["b11".to_string(), "b12".to_string()],
-            fields_type: vec![TableDataType::Boolean, TableDataType::String],
-        };
-        let b = TableDataType::Tuple {
-            fields_name: vec!["b1".to_string(), "b2".to_string()],
-            fields_type: vec![b1, TableDataType::Number(NumberDataType::Int64)],
-        };
-        let fields = vec![
-            TableField::new("a", TableDataType::Number(NumberDataType::UInt64)),
-            TableField::new("b", b),
-            TableField::new("c", TableDataType::Number(NumberDataType::UInt64)),
-        ];
-        let schema = TableSchema::new(fields);
-
+        let schema = new_latest_schema();
         let p = schema.to_pb()?;
 
         let mut buf = vec![];

src/meta/proto-conv/tests/it/v027_schema.rs

@@ -0,0 +1,83 @@
+// Copyright 2023 Datafuse Labs.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+use common_expression::types::decimal::DecimalSize;
+use common_expression::types::DecimalDataType;
+use common_expression::types::NumberDataType;
+use common_expression::TableDataType;
+use common_expression::TableField;
+use common_expression::TableSchema;
+
+use crate::common;
+
+// These bytes are built when a new version in introduced,
+// and are kept for backward compatibility test.
+//
+// *************************************************************
+// * These messages should never be updated,                   *
+// * only be added when a new version is added,                *
+// * or be removed when an old version is no longer supported. *
+// *************************************************************
+//
+// The message bytes are built from the output of `test_build_pb_buf()`
+#[test]
+fn test_decode_v27_schema() -> anyhow::Result<()> {
+    let schema_v27 = [
+        10, 28, 10, 1, 97, 26, 17, 154, 2, 8, 34, 0, 160, 6, 27, 168, 6, 24, 160, 6, 27, 168, 6,
+        24, 160, 6, 27, 168, 6, 24, 10, 104, 10, 1, 98, 26, 91, 202, 2, 82, 10, 2, 98, 49, 10, 2,
+        98, 50, 18, 47, 202, 2, 38, 10, 3, 98, 49, 49, 10, 3, 98, 49, 50, 18, 9, 138, 2, 0, 160, 6,
+        27, 168, 6, 24, 18, 9, 146, 2, 0, 160, 6, 27, 168, 6, 24, 160, 6, 27, 168, 6, 24, 160, 6,
+        27, 168, 6, 24, 18, 17, 154, 2, 8, 66, 0, 160, 6, 27, 168, 6, 24, 160, 6, 27, 168, 6, 24,
+        160, 6, 27, 168, 6, 24, 160, 6, 27, 168, 6, 24, 32, 1, 160, 6, 27, 168, 6, 24, 10, 30, 10,
+        1, 99, 26, 17, 154, 2, 8, 34, 0, 160, 6, 27, 168, 6, 24, 160, 6, 27, 168, 6, 24, 32, 4,
+        160, 6, 27, 168, 6, 24, 10, 49, 10, 10, 100, 101, 99, 105, 109, 97, 108, 49, 50, 56, 26,
+        27, 218, 2, 18, 10, 10, 8, 18, 16, 3, 160, 6, 27, 168, 6, 24, 160, 6, 27, 168, 6, 24, 160,
+        6, 27, 168, 6, 24, 32, 5, 160, 6, 27, 168, 6, 24, 10, 49, 10, 10, 100, 101, 99, 105, 109,
+        97, 108, 50, 53, 54, 26, 27, 218, 2, 18, 18, 10, 8, 46, 16, 6, 160, 6, 27, 168, 6, 24, 160,
+        6, 27, 168, 6, 24, 160, 6, 27, 168, 6, 24, 32, 6, 160, 6, 27, 168, 6, 24, 24, 7, 160, 6,
+        27, 168, 6, 24,
+    ];
+
+    let b1 = TableDataType::Tuple {
+        fields_name: vec!["b11".to_string(), "b12".to_string()],
+        fields_type: vec![TableDataType::Boolean, TableDataType::String],
+    };
+    let b = TableDataType::Tuple {
+        fields_name: vec!["b1".to_string(), "b2".to_string()],
+        fields_type: vec![b1, TableDataType::Number(NumberDataType::Int64)],
+    };
+    let fields = vec![
+        TableField::new("a", TableDataType::Number(NumberDataType::UInt64)),
+        TableField::new("b", b),
+        TableField::new("c", TableDataType::Number(NumberDataType::UInt64)),
+        TableField::new(
+            "decimal128",
+            TableDataType::Decimal(DecimalDataType::Decimal128(DecimalSize {
+                precision: 18,
+                scale: 3,
+            })),
+        ),
+        TableField::new(
+            "decimal256",
+            TableDataType::Decimal(DecimalDataType::Decimal256(DecimalSize {
+                precision: 46,
+                scale: 6,
+            })),
+        ),
+    ];
+    let want = || TableSchema::new(fields.clone());
+    common::test_load_old(func_name!(), schema_v27.as_slice(), 27, want())?;
+    common::test_pb_from_to(func_name!(), want())?;
+    Ok(())
+}

src/query/expression/src/kernels/group_by_hash.rs

@@ -38,8 +38,8 @@ use crate::types::DataType;
 use crate::types::NumberDataType;
 use crate::types::NumberType;
 use crate::types::ValueType;
+use crate::with_integer_mapped_type;
 use crate::with_number_mapped_type;
-use crate::with_unsigned_number_mapped_type;
 use crate::Column;
 use crate::TypeDeserializer;
 
@@ -274,10 +274,10 @@ where T: Clone
     ) -> Result<Vec<Column>> {
         debug_assert!(!keys.is_empty());
 
-        // faster path for single unsigned integer to column
-        if group_items.len() == 1 && group_items[0].1.is_unsigned_numeric() {
+        // faster path for single signed/unsigned integer to column
+        if group_items.len() == 1 && group_items[0].1.is_numeric() {
             if let DataType::Number(ty) = group_items[0].1 {
-                with_unsigned_number_mapped_type!(|NUM_TYPE| match ty {
+                with_integer_mapped_type!(|NUM_TYPE| match ty {
                     NumberDataType::NUM_TYPE => {
                         let buffer: Buffer<T> = keys.into();
                         let col =
@@ -374,7 +374,7 @@ where T: Clone
 }
 
 macro_rules! impl_hash_method_fixed_keys {
-    ($dt: ident, $ty:ty) => {
+    ($dt: ident, $ty:ty, $signed_ty: ty) => {
         impl HashMethod for HashMethodFixedKeys<$ty> {
             type HashKey = $ty;
             type HashKeyIter<'a> = std::slice::Iter<'a, $ty>;
@@ -389,9 +389,21 @@ macro_rules! impl_hash_method_fixed_keys {
                 rows: usize,
             ) -> Result<KeysState> {
                 // faster path for single fixed keys
-                if group_columns.len() == 1 && group_columns[0].1.is_unsigned_numeric() {
-                    return Ok(KeysState::Column(group_columns[0].0.clone()));
+                if group_columns.len() == 1 {
+                    if group_columns[0].1.is_unsigned_numeric() {
+                        return Ok(KeysState::Column(group_columns[0].0.clone()));
+                    }
+
+                    if group_columns[0].1.is_signed_numeric() {
+                        let col =
+                            NumberType::<$signed_ty>::try_downcast_column(&group_columns[0].0)
+                                .unwrap();
+                        let buffer =
+                            unsafe { std::mem::transmute::<Buffer<$signed_ty>, Buffer<$ty>>(col) };
+                        return Ok(KeysState::Column(NumberType::<$ty>::upcast_column(buffer)));
+                    }
                 }
+
                 let keys = self.build_keys_vec(group_columns, rows)?;
                 let col = Buffer::<$ty>::from(keys);
                 Ok(KeysState::Column(NumberType::<$ty>::upcast_column(col)))
@@ -411,10 +423,10 @@ macro_rules! impl_hash_method_fixed_keys {
     };
 }
 
-impl_hash_method_fixed_keys! {UInt8, u8}
-impl_hash_method_fixed_keys! {UInt16, u16}
-impl_hash_method_fixed_keys! {UInt32, u32}
-impl_hash_method_fixed_keys! {UInt64, u64}
+impl_hash_method_fixed_keys! {UInt8, u8, i8}
+impl_hash_method_fixed_keys! {UInt16, u16, i16}
+impl_hash_method_fixed_keys! {UInt32, u32, i32}
+impl_hash_method_fixed_keys! {UInt64, u64, i64}
 
 macro_rules! impl_hash_method_fixed_large_keys {
     ($ty:ty, $name: ident) => {

src/query/expression/src/types.rs

@@ -121,6 +121,15 @@ impl DataType {
         }
     }
 
+    pub fn is_signed_numeric(&self) -> bool {
+        match self {
+            DataType::Number(ty) => {
+                ALL_INTEGER_TYPES.contains(ty) && !ALL_UNSIGNED_INTEGER_TYPES.contains(ty)
+            }
+            _ => false,
+        }
+    }
+
     pub fn is_numeric(&self) -> bool {
         match self {
             DataType::Number(ty) => ALL_NUMERICS_TYPES.contains(ty),