lide-reed commented on code in PR #38560: URL: https://github.com/apache/doris/pull/38560#discussion_r1699287705
########## be/src/vec/functions/array/function_array_distinct.h: ########## @@ -257,63 +254,38 @@ class FunctionArrayDistinct : public IFunction { IColumn& dest_column, ColumnArray::Offsets64& dest_offsets, const NullMapType* src_null_map, NullMapType* dest_null_map, DataTypePtr& nested_type) const { +#define EXECUTE_NUMBER(TYPE, NAME) \ + if (which.is_##NAME()) { \ + res = _execute_number<TYPE>(src_column, src_offsets, dest_column, dest_offsets, \ + src_null_map, dest_null_map); \ + } else + bool res = false; WhichDataType which(remove_nullable(nested_type)); - if (which.is_uint8()) { - res = _execute_number<ColumnUInt8>(src_column, src_offsets, dest_column, dest_offsets, - src_null_map, dest_null_map); - } else if (which.is_int8()) { - res = _execute_number<ColumnInt8>(src_column, src_offsets, dest_column, dest_offsets, - src_null_map, dest_null_map); - } else if (which.is_int16()) { - res = _execute_number<ColumnInt16>(src_column, src_offsets, dest_column, dest_offsets, - src_null_map, dest_null_map); - } else if (which.is_int32()) { - res = _execute_number<ColumnInt32>(src_column, src_offsets, dest_column, dest_offsets, - src_null_map, dest_null_map); - } else if (which.is_int64()) { - res = _execute_number<ColumnInt64>(src_column, src_offsets, dest_column, dest_offsets, - src_null_map, dest_null_map); - } else if (which.is_int128()) { - res = _execute_number<ColumnInt128>(src_column, src_offsets, dest_column, dest_offsets, - src_null_map, dest_null_map); - } else if (which.is_float32()) { - res = _execute_number<ColumnFloat32>(src_column, src_offsets, dest_column, dest_offsets, - src_null_map, dest_null_map); - } else if (which.is_float64()) { - res = _execute_number<ColumnFloat64>(src_column, src_offsets, dest_column, dest_offsets, - src_null_map, dest_null_map); - } else if (which.is_date()) { - res = _execute_number<ColumnDate>(src_column, src_offsets, dest_column, dest_offsets, - src_null_map, dest_null_map); - } else if (which.is_date_time()) { - res = _execute_number<ColumnDateTime>(src_column, src_offsets, dest_column, - dest_offsets, src_null_map, dest_null_map); - } else if (which.is_date_v2()) { - res = _execute_number<ColumnDateV2>(src_column, src_offsets, dest_column, dest_offsets, - src_null_map, dest_null_map); - } else if (which.is_date_time_v2()) { - res = _execute_number<ColumnDateTimeV2>(src_column, src_offsets, dest_column, - dest_offsets, src_null_map, dest_null_map); - } else if (which.is_decimal32()) { - res = _execute_number<ColumnDecimal32>(src_column, src_offsets, dest_column, - dest_offsets, src_null_map, dest_null_map); - } else if (which.is_decimal64()) { - res = _execute_number<ColumnDecimal64>(src_column, src_offsets, dest_column, - dest_offsets, src_null_map, dest_null_map); - } else if (which.is_decimal128v3()) { - res = _execute_number<ColumnDecimal128V3>(src_column, src_offsets, dest_column, - dest_offsets, src_null_map, dest_null_map); - } else if (which.is_decimal256()) { - res = _execute_number<ColumnDecimal256>(src_column, src_offsets, dest_column, - dest_offsets, src_null_map, dest_null_map); - } else if (which.is_decimal128v2()) { - res = _execute_number<ColumnDecimal128V2>(src_column, src_offsets, dest_column, - dest_offsets, src_null_map, dest_null_map); - } else if (which.is_string()) { + EXECUTE_NUMBER(ColumnUInt8, uint8) + EXECUTE_NUMBER(ColumnInt8, int8) + EXECUTE_NUMBER(ColumnInt16, int16) + EXECUTE_NUMBER(ColumnInt32, int32) + EXECUTE_NUMBER(ColumnInt64, int64) + EXECUTE_NUMBER(ColumnInt128, int128) + EXECUTE_NUMBER(ColumnFloat32, float32) + EXECUTE_NUMBER(ColumnFloat64, float64) + EXECUTE_NUMBER(ColumnDate, date) + EXECUTE_NUMBER(ColumnDateTime, date_time) + EXECUTE_NUMBER(ColumnDateV2, date_v2) + EXECUTE_NUMBER(ColumnDateTimeV2, date_time_v2) + EXECUTE_NUMBER(ColumnDecimal32, decimal32) + EXECUTE_NUMBER(ColumnDecimal64, decimal64) + EXECUTE_NUMBER(ColumnDecimal128V3, decimal128v3) + EXECUTE_NUMBER(ColumnDecimal256, decimal256) + EXECUTE_NUMBER(ColumnDecimal128V2, decimal128v2) + if (which.is_string()) { res = _execute_string(src_column, src_offsets, dest_column, dest_offsets, src_null_map, dest_null_map); } + +#undef EXECUTE_NUMBER + return res; Review Comment: If all conditions missed, we should print some warning log? ########## be/src/vec/functions/array/function_array_distinct.h: ########## @@ -257,63 +254,38 @@ class FunctionArrayDistinct : public IFunction { IColumn& dest_column, ColumnArray::Offsets64& dest_offsets, const NullMapType* src_null_map, NullMapType* dest_null_map, DataTypePtr& nested_type) const { +#define EXECUTE_NUMBER(TYPE, NAME) \ + if (which.is_##NAME()) { \ + res = _execute_number<TYPE>(src_column, src_offsets, dest_column, dest_offsets, \ + src_null_map, dest_null_map); \ + } else + Review Comment: making it more readable and reducing the need for additional else statements, and using return as following: #define EXECUTE_NUMBER(TYPE, NAME) \ if (which.is_##NAME()) { \ res = _execute_number<TYPE>(src_column, src_offsets, dest_column, dest_offsets, src_null_map, dest_null_map); \ return res; \ } ########## be/src/vec/functions/array/function_array_distinct.h: ########## @@ -254,13 +254,12 @@ class FunctionArrayDistinct : public IFunction { IColumn& dest_column, ColumnArray::Offsets64& dest_offsets, const NullMapType* src_null_map, NullMapType* dest_null_map, DataTypePtr& nested_type) const { -#define EXECUTE_NUMBER(TYPE, NAME) \ - if (which.is_##NAME()) { \ - res = _execute_number<TYPE>(src_column, src_offsets, dest_column, dest_offsets, \ - src_null_map, dest_null_map); \ +#define EXECUTE_NUMBER(TYPE, NAME) \ + if (which.is_##NAME()) { \ + return _execute_number<TYPE>(src_column, src_offsets, dest_column, dest_offsets, \ + src_null_map, dest_null_map); \ } else Review Comment: The last else should be removed ########## be/src/vec/functions/array/function_array_distinct.h: ########## @@ -257,64 +254,41 @@ class FunctionArrayDistinct : public IFunction { IColumn& dest_column, ColumnArray::Offsets64& dest_offsets, const NullMapType* src_null_map, NullMapType* dest_null_map, DataTypePtr& nested_type) const { - bool res = false; +#define EXECUTE_NUMBER(TYPE, NAME) \ + if (which.is_##NAME()) { \ + return _execute_number<TYPE>(src_column, src_offsets, dest_column, dest_offsets, \ + src_null_map, dest_null_map); \ + } + WhichDataType which(remove_nullable(nested_type)); - if (which.is_uint8()) { - res = _execute_number<ColumnUInt8>(src_column, src_offsets, dest_column, dest_offsets, - src_null_map, dest_null_map); - } else if (which.is_int8()) { - res = _execute_number<ColumnInt8>(src_column, src_offsets, dest_column, dest_offsets, - src_null_map, dest_null_map); - } else if (which.is_int16()) { - res = _execute_number<ColumnInt16>(src_column, src_offsets, dest_column, dest_offsets, - src_null_map, dest_null_map); - } else if (which.is_int32()) { - res = _execute_number<ColumnInt32>(src_column, src_offsets, dest_column, dest_offsets, - src_null_map, dest_null_map); - } else if (which.is_int64()) { - res = _execute_number<ColumnInt64>(src_column, src_offsets, dest_column, dest_offsets, - src_null_map, dest_null_map); - } else if (which.is_int128()) { - res = _execute_number<ColumnInt128>(src_column, src_offsets, dest_column, dest_offsets, - src_null_map, dest_null_map); - } else if (which.is_float32()) { - res = _execute_number<ColumnFloat32>(src_column, src_offsets, dest_column, dest_offsets, - src_null_map, dest_null_map); - } else if (which.is_float64()) { - res = _execute_number<ColumnFloat64>(src_column, src_offsets, dest_column, dest_offsets, - src_null_map, dest_null_map); - } else if (which.is_date()) { - res = _execute_number<ColumnDate>(src_column, src_offsets, dest_column, dest_offsets, - src_null_map, dest_null_map); - } else if (which.is_date_time()) { - res = _execute_number<ColumnDateTime>(src_column, src_offsets, dest_column, - dest_offsets, src_null_map, dest_null_map); - } else if (which.is_date_v2()) { - res = _execute_number<ColumnDateV2>(src_column, src_offsets, dest_column, dest_offsets, - src_null_map, dest_null_map); - } else if (which.is_date_time_v2()) { - res = _execute_number<ColumnDateTimeV2>(src_column, src_offsets, dest_column, - dest_offsets, src_null_map, dest_null_map); - } else if (which.is_decimal32()) { - res = _execute_number<ColumnDecimal32>(src_column, src_offsets, dest_column, - dest_offsets, src_null_map, dest_null_map); - } else if (which.is_decimal64()) { - res = _execute_number<ColumnDecimal64>(src_column, src_offsets, dest_column, - dest_offsets, src_null_map, dest_null_map); - } else if (which.is_decimal128v3()) { - res = _execute_number<ColumnDecimal128V3>(src_column, src_offsets, dest_column, - dest_offsets, src_null_map, dest_null_map); - } else if (which.is_decimal256()) { - res = _execute_number<ColumnDecimal256>(src_column, src_offsets, dest_column, - dest_offsets, src_null_map, dest_null_map); - } else if (which.is_decimal128v2()) { - res = _execute_number<ColumnDecimal128V2>(src_column, src_offsets, dest_column, - dest_offsets, src_null_map, dest_null_map); - } else if (which.is_string()) { - res = _execute_string(src_column, src_offsets, dest_column, dest_offsets, src_null_map, - dest_null_map); + EXECUTE_NUMBER(ColumnUInt8, uint8) Review Comment: Please add ";" to the end of each line -- 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-unsubscr...@doris.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org For additional commands, e-mail: commits-h...@doris.apache.org