HappenLee commented on code in PR #35628: URL: https://github.com/apache/doris/pull/35628#discussion_r1625900757
########## regression-test/suites/mv_p0/agg_state/diffrent_serialize/diffrent_serialize.groovy: ########## @@ -0,0 +1,93 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you 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. + +import org.codehaus.groovy.runtime.IOGroovyMethods + +suite ("diffrent_serialize") { + + sql """ DROP TABLE IF EXISTS d_table; """ + + sql """ + create table d_table( + k1 int null, + k2 int not null, + k3 bigint null, + k4 varchar(100) null + ) + duplicate key (k1,k2,k3) + distributed BY hash(k1) buckets 3 + properties("replication_num" = "1"); + """ + + sql "insert into d_table select 1,1,1,'a';" + sql "insert into d_table select 2,2,2,'b';" + sql "insert into d_table select 3,3,null,'c';" + + createMV("create materialized view mv1 as select k1,bitmap_agg(k2) from d_table group by k1;") + /* + createMV("create materialized view mv2 as select k1,map_agg(k2,k3) from d_table group by k1;") + createMV("create materialized view mv3 as select k1,array_agg(k2) from d_table group by k1;") + createMV("create materialized view mv4 as select k1,collect_list(k2,3) from d_table group by k1;") + createMV("create materialized view mv5 as select k1,collect_set(k2,3) from d_table group by k1;") + */ + + sql "insert into d_table select -4,4,-4,'d';" + sql "insert into d_table(k4,k2) values('d',4);" + + qt_select_star "select * from d_table order by k1;" + + explain { + sql("select k1,bitmap_to_string(bitmap_agg(k2)) from d_table group by k1 order by 1;") + contains "(mv1)" + } + qt_select_mv "select k1,bitmap_to_string(bitmap_agg(k2)) from d_table group by k1 order by 1;" + + sql "insert into d_table select 1,1,1,'a';" + sql "insert into d_table select 1,2,1,'a';" + + explain { + sql("select k1,bitmap_count(bitmap_agg(k2)) from d_table group by k1 order by 1;") + contains "(mv1)" + } + qt_select_mv "select k1,bitmap_count(bitmap_agg(k2)) from d_table group by k1 order by 1;" + +/* Review Comment: why comment the case? ########## be/src/vec/olap/olap_data_convertor.cpp: ########## @@ -1045,15 +1054,17 @@ Status OlapBlockDataConvertor::OlapColumnDataConvertorMap::convert_to_olap( _offsets.push_back(column_map->offset_at(i + _row_pos) - start_offset + _base_offset); } _base_offset += elem_size; - ColumnWithTypeAndName key_typed_column = {key_data, data_type_map->get_key_type(), "map.key"}; + ColumnWithTypeAndName key_typed_column = {key_data, _data_type.get_key_type(), "map.key"}; _key_convertor->set_source_column(key_typed_column, start_offset, elem_size); RETURN_IF_ERROR(_key_convertor->convert_to_olap()); - ColumnWithTypeAndName value_typed_column = {value_data, data_type_map->get_value_type(), + ColumnWithTypeAndName value_typed_column = {value_data, _data_type.get_value_type(), "map.value"}; _value_convertor->set_source_column(value_typed_column, start_offset, elem_size); RETURN_IF_ERROR(_value_convertor->convert_to_olap()); + LOG(WARNING) << _data_type.get_name(); Review Comment: seems dubeg logging, delete it ########## be/src/vec/olap/olap_data_convertor.cpp: ########## @@ -181,23 +202,17 @@ OlapBlockDataConvertor::create_olap_column_data_convertor(const TabletColumn& co return std::make_unique<OlapColumnDataConvertorStruct>(sub_convertors); } case FieldType::OLAP_FIELD_TYPE_ARRAY: { - const auto& sub_column = column.get_sub_column(0); - return std::make_unique<OlapColumnDataConvertorArray>( - create_olap_column_data_convertor(sub_column)); + return create_array_convertor(column); } case FieldType::OLAP_FIELD_TYPE_MAP: { - const auto& key_column = column.get_sub_column(0); - const auto& value_column = column.get_sub_column(1); - return std::make_unique<OlapColumnDataConvertorMap>( - create_olap_column_data_convertor(key_column), - create_olap_column_data_convertor(value_column)); + return create_map_convertor(column); } default: { Review Comment: no need the the `default` if do nothing ########## be/src/vec/olap/olap_data_convertor.cpp: ########## @@ -74,6 +73,39 @@ void OlapBlockDataConvertor::add_column_data_convertor(const TabletColumn& colum _convertors.emplace_back(create_olap_column_data_convertor(column)); } +OlapBlockDataConvertor::OlapColumnDataConvertorBaseUPtr +OlapBlockDataConvertor::create_map_convertor(const TabletColumn& column) { + const auto& key_column = column.get_sub_column(0); + const auto& value_column = column.get_sub_column(1); + return std::make_unique<OlapColumnDataConvertorMap>(key_column, value_column); +} + +OlapBlockDataConvertor::OlapColumnDataConvertorBaseUPtr +OlapBlockDataConvertor::create_array_convertor(const TabletColumn& column) { + const auto& sub_column = column.get_sub_column(0); + return std::make_unique<OlapColumnDataConvertorArray>( + create_olap_column_data_convertor(sub_column)); +} + +OlapBlockDataConvertor::OlapColumnDataConvertorBaseUPtr +OlapBlockDataConvertor::create_agg_state_convertor(const TabletColumn& column) { + auto data_type = DataTypeFactory::instance().create_data_type(column); + const auto* agg_state_type = assert_cast<const vectorized::DataTypeAggState*>(data_type.get()); + auto type = agg_state_type->get_serialized_type()->get_type_as_type_descriptor().type; + + if (type == PrimitiveType::TYPE_STRING) { + return std::make_unique<OlapColumnDataConvertorVarChar>(false); + } else if (type == PrimitiveType::TYPE_OBJECT) { + return std::make_unique<OlapColumnDataConvertorBitMap>(); + } else if (type == PrimitiveType::INVALID_TYPE) { Review Comment: """ The logic appears rather strange and requires a significant amount of commentary. """ ########## be/src/vec/data_types/data_type_factory.cpp: ########## @@ -604,7 +619,7 @@ DataTypePtr DataTypeFactory::create_data_type(const segment_v2::ColumnMetaPB& pc names.reserve(col_size); for (size_t i = 0; i < col_size; i++) { dataTypes.push_back(create_data_type(pcolumn.children_columns(i))); - names.push_back(""); + names.emplace_back(""); Review Comment: just resize and not need call `emplace_back` -- 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
