Skip to content

Commit dd27715

Browse files
sundy-lisundy-li
authored
Merge pull request #9978 from sundy-li/group-1
2 parents 5a5267e + 37e014f commit dd27715

File tree

5 files changed

+150
-26
lines changed

5 files changed

+150
-26
lines changed

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,3 +33,4 @@ mod v023_table_meta;
3333
mod v024_table_meta;
3434
mod v025_user_stage;
3535
mod v026_schema;
36+
mod v027_schema;

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

Lines changed: 34 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ use std::collections::BTreeMap;
1616
use std::collections::BTreeSet;
1717
use std::sync::Arc;
1818

19+
use ce::types::decimal::DecimalSize;
20+
use ce::types::DecimalDataType;
1921
use ce::types::NumberDataType;
2022
use chrono::TimeZone;
2123
use chrono::Utc;
@@ -196,6 +198,37 @@ fn new_table_meta() -> mt::TableMeta {
196198
}
197199
}
198200

201+
pub(crate) fn new_latest_schema() -> TableSchema {
202+
let b1 = TableDataType::Tuple {
203+
fields_name: vec!["b11".to_string(), "b12".to_string()],
204+
fields_type: vec![TableDataType::Boolean, TableDataType::String],
205+
};
206+
let b = TableDataType::Tuple {
207+
fields_name: vec!["b1".to_string(), "b2".to_string()],
208+
fields_type: vec![b1, TableDataType::Number(NumberDataType::Int64)],
209+
};
210+
let fields = vec![
211+
TableField::new("a", TableDataType::Number(NumberDataType::UInt64)),
212+
TableField::new("b", b),
213+
TableField::new("c", TableDataType::Number(NumberDataType::UInt64)),
214+
TableField::new(
215+
"decimal128",
216+
TableDataType::Decimal(DecimalDataType::Decimal128(DecimalSize {
217+
precision: 18,
218+
scale: 3,
219+
})),
220+
),
221+
ce::TableField::new(
222+
"decimal256",
223+
TableDataType::Decimal(DecimalDataType::Decimal256(DecimalSize {
224+
precision: 46,
225+
scale: 6,
226+
})),
227+
),
228+
];
229+
TableSchema::new(fields)
230+
}
231+
199232
pub(crate) fn new_table_copied_file_info_v6() -> mt::TableCopiedFileInfo {
200233
mt::TableCopiedFileInfo {
201234
etag: Some("etag".to_string()),
@@ -338,21 +371,7 @@ fn test_build_pb_buf() -> anyhow::Result<()> {
338371

339372
// schema
340373
{
341-
let b1 = TableDataType::Tuple {
342-
fields_name: vec!["b11".to_string(), "b12".to_string()],
343-
fields_type: vec![TableDataType::Boolean, TableDataType::String],
344-
};
345-
let b = TableDataType::Tuple {
346-
fields_name: vec!["b1".to_string(), "b2".to_string()],
347-
fields_type: vec![b1, TableDataType::Number(NumberDataType::Int64)],
348-
};
349-
let fields = vec![
350-
TableField::new("a", TableDataType::Number(NumberDataType::UInt64)),
351-
TableField::new("b", b),
352-
TableField::new("c", TableDataType::Number(NumberDataType::UInt64)),
353-
];
354-
let schema = TableSchema::new(fields);
355-
374+
let schema = new_latest_schema();
356375
let p = schema.to_pb()?;
357376

358377
let mut buf = vec![];

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

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
// Copyright 2023 Datafuse Labs.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
use common_expression::types::decimal::DecimalSize;
16+
use common_expression::types::DecimalDataType;
17+
use common_expression::types::NumberDataType;
18+
use common_expression::TableDataType;
19+
use common_expression::TableField;
20+
use common_expression::TableSchema;
21+
22+
use crate::common;
23+
24+
// These bytes are built when a new version in introduced,
25+
// and are kept for backward compatibility test.
26+
//
27+
// *************************************************************
28+
// * These messages should never be updated, *
29+
// * only be added when a new version is added, *
30+
// * or be removed when an old version is no longer supported. *
31+
// *************************************************************
32+
//
33+
// The message bytes are built from the output of `test_build_pb_buf()`
34+
#[test]
35+
fn test_decode_v27_schema() -> anyhow::Result<()> {
36+
let schema_v27 = [
37+
10, 28, 10, 1, 97, 26, 17, 154, 2, 8, 34, 0, 160, 6, 27, 168, 6, 24, 160, 6, 27, 168, 6,
38+
24, 160, 6, 27, 168, 6, 24, 10, 104, 10, 1, 98, 26, 91, 202, 2, 82, 10, 2, 98, 49, 10, 2,
39+
98, 50, 18, 47, 202, 2, 38, 10, 3, 98, 49, 49, 10, 3, 98, 49, 50, 18, 9, 138, 2, 0, 160, 6,
40+
27, 168, 6, 24, 18, 9, 146, 2, 0, 160, 6, 27, 168, 6, 24, 160, 6, 27, 168, 6, 24, 160, 6,
41+
27, 168, 6, 24, 18, 17, 154, 2, 8, 66, 0, 160, 6, 27, 168, 6, 24, 160, 6, 27, 168, 6, 24,
42+
160, 6, 27, 168, 6, 24, 160, 6, 27, 168, 6, 24, 32, 1, 160, 6, 27, 168, 6, 24, 10, 30, 10,
43+
1, 99, 26, 17, 154, 2, 8, 34, 0, 160, 6, 27, 168, 6, 24, 160, 6, 27, 168, 6, 24, 32, 4,
44+
160, 6, 27, 168, 6, 24, 10, 49, 10, 10, 100, 101, 99, 105, 109, 97, 108, 49, 50, 56, 26,
45+
27, 218, 2, 18, 10, 10, 8, 18, 16, 3, 160, 6, 27, 168, 6, 24, 160, 6, 27, 168, 6, 24, 160,
46+
6, 27, 168, 6, 24, 32, 5, 160, 6, 27, 168, 6, 24, 10, 49, 10, 10, 100, 101, 99, 105, 109,
47+
97, 108, 50, 53, 54, 26, 27, 218, 2, 18, 18, 10, 8, 46, 16, 6, 160, 6, 27, 168, 6, 24, 160,
48+
6, 27, 168, 6, 24, 160, 6, 27, 168, 6, 24, 32, 6, 160, 6, 27, 168, 6, 24, 24, 7, 160, 6,
49+
27, 168, 6, 24,
50+
];
51+
52+
let b1 = TableDataType::Tuple {
53+
fields_name: vec!["b11".to_string(), "b12".to_string()],
54+
fields_type: vec![TableDataType::Boolean, TableDataType::String],
55+
};
56+
let b = TableDataType::Tuple {
57+
fields_name: vec!["b1".to_string(), "b2".to_string()],
58+
fields_type: vec![b1, TableDataType::Number(NumberDataType::Int64)],
59+
};
60+
let fields = vec![
61+
TableField::new("a", TableDataType::Number(NumberDataType::UInt64)),
62+
TableField::new("b", b),
63+
TableField::new("c", TableDataType::Number(NumberDataType::UInt64)),
64+
TableField::new(
65+
"decimal128",
66+
TableDataType::Decimal(DecimalDataType::Decimal128(DecimalSize {
67+
precision: 18,
68+
scale: 3,
69+
})),
70+
),
71+
TableField::new(
72+
"decimal256",
73+
TableDataType::Decimal(DecimalDataType::Decimal256(DecimalSize {
74+
precision: 46,
75+
scale: 6,
76+
})),
77+
),
78+
];
79+
let want = || TableSchema::new(fields.clone());
80+
common::test_load_old(func_name!(), schema_v27.as_slice(), 27, want())?;
81+
common::test_pb_from_to(func_name!(), want())?;
82+
Ok(())
83+
}

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

Lines changed: 23 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,8 @@ use crate::types::DataType;
3838
use crate::types::NumberDataType;
3939
use crate::types::NumberType;
4040
use crate::types::ValueType;
41+
use crate::with_integer_mapped_type;
4142
use crate::with_number_mapped_type;
42-
use crate::with_unsigned_number_mapped_type;
4343
use crate::Column;
4444
use crate::TypeDeserializer;
4545

@@ -274,10 +274,10 @@ where T: Clone
274274
) -> Result<Vec<Column>> {
275275
debug_assert!(!keys.is_empty());
276276

277-
// faster path for single unsigned integer to column
278-
if group_items.len() == 1 && group_items[0].1.is_unsigned_numeric() {
277+
// faster path for single signed/unsigned integer to column
278+
if group_items.len() == 1 && group_items[0].1.is_numeric() {
279279
if let DataType::Number(ty) = group_items[0].1 {
280-
with_unsigned_number_mapped_type!(|NUM_TYPE| match ty {
280+
with_integer_mapped_type!(|NUM_TYPE| match ty {
281281
NumberDataType::NUM_TYPE => {
282282
let buffer: Buffer<T> = keys.into();
283283
let col =
@@ -374,7 +374,7 @@ where T: Clone
374374
}
375375

376376
macro_rules! impl_hash_method_fixed_keys {
377-
($dt: ident, $ty:ty) => {
377+
($dt: ident, $ty:ty, $signed_ty: ty) => {
378378
impl HashMethod for HashMethodFixedKeys<$ty> {
379379
type HashKey = $ty;
380380
type HashKeyIter<'a> = std::slice::Iter<'a, $ty>;
@@ -389,9 +389,21 @@ macro_rules! impl_hash_method_fixed_keys {
389389
rows: usize,
390390
) -> Result<KeysState> {
391391
// faster path for single fixed keys
392-
if group_columns.len() == 1 && group_columns[0].1.is_unsigned_numeric() {
393-
return Ok(KeysState::Column(group_columns[0].0.clone()));
392+
if group_columns.len() == 1 {
393+
if group_columns[0].1.is_unsigned_numeric() {
394+
return Ok(KeysState::Column(group_columns[0].0.clone()));
395+
}
396+
397+
if group_columns[0].1.is_signed_numeric() {
398+
let col =
399+
NumberType::<$signed_ty>::try_downcast_column(&group_columns[0].0)
400+
.unwrap();
401+
let buffer =
402+
unsafe { std::mem::transmute::<Buffer<$signed_ty>, Buffer<$ty>>(col) };
403+
return Ok(KeysState::Column(NumberType::<$ty>::upcast_column(buffer)));
404+
}
394405
}
406+
395407
let keys = self.build_keys_vec(group_columns, rows)?;
396408
let col = Buffer::<$ty>::from(keys);
397409
Ok(KeysState::Column(NumberType::<$ty>::upcast_column(col)))
@@ -411,10 +423,10 @@ macro_rules! impl_hash_method_fixed_keys {
411423
};
412424
}
413425

414-
impl_hash_method_fixed_keys! {UInt8, u8}
415-
impl_hash_method_fixed_keys! {UInt16, u16}
416-
impl_hash_method_fixed_keys! {UInt32, u32}
417-
impl_hash_method_fixed_keys! {UInt64, u64}
426+
impl_hash_method_fixed_keys! {UInt8, u8, i8}
427+
impl_hash_method_fixed_keys! {UInt16, u16, i16}
428+
impl_hash_method_fixed_keys! {UInt32, u32, i32}
429+
impl_hash_method_fixed_keys! {UInt64, u64, i64}
418430

419431
macro_rules! impl_hash_method_fixed_large_keys {
420432
($ty:ty, $name: ident) => {

‎src/query/expression/src/types.rs‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,15 @@ impl DataType {
121121
}
122122
}
123123

124+
pub fn is_signed_numeric(&self) -> bool {
125+
match self {
126+
DataType::Number(ty) => {
127+
ALL_INTEGER_TYPES.contains(ty) && !ALL_UNSIGNED_INTEGER_TYPES.contains(ty)
128+
}
129+
_ => false,
130+
}
131+
}
132+
124133
pub fn is_numeric(&self) -> bool {
125134
match self {
126135
DataType::Number(ty) => ALL_NUMERICS_TYPES.contains(ty),

0 commit comments

Comments
 (0)