zclllyybb commented on code in PR #48398: URL: https://github.com/apache/doris/pull/48398#discussion_r1976667430
########## be/test/vec/data_types/serde/data_type_serde_arrow_test.cpp: ########## @@ -86,13 +86,15 @@ void serialize_and_deserialize_arrow_test() { {"k7", FieldType::OLAP_FIELD_TYPE_INT, 7, TYPE_INT, true}, {"k2", FieldType::OLAP_FIELD_TYPE_STRING, 2, TYPE_STRING, false}, {"k3", FieldType::OLAP_FIELD_TYPE_DECIMAL128I, 3, TYPE_DECIMAL128I, false}, - {"k11", FieldType::OLAP_FIELD_TYPE_DATETIME, 11, TYPE_DATETIME, false}, {"k4", FieldType::OLAP_FIELD_TYPE_BOOL, 4, TYPE_BOOLEAN, false}, {"k5", FieldType::OLAP_FIELD_TYPE_DECIMAL32, 5, TYPE_DECIMAL32, false}, {"k6", FieldType::OLAP_FIELD_TYPE_DECIMAL64, 6, TYPE_DECIMAL64, false}, - {"k12", FieldType::OLAP_FIELD_TYPE_DATETIMEV2, 12, TYPE_DATETIMEV2, false}, {"k8", FieldType::OLAP_FIELD_TYPE_IPV4, 8, TYPE_IPV4, false}, {"k9", FieldType::OLAP_FIELD_TYPE_IPV6, 9, TYPE_IPV6, false}, + {"k11", FieldType::OLAP_FIELD_TYPE_DATETIME, 11, TYPE_DATETIME, false}, Review Comment: 这个 `is_scalar` 如果只是决定构造什么cols的话,放在这里感觉没啥道理。最好去掉这个模板参数,转而把 `cols` 变成一个入参。 ########## be/src/runtime/types.h: ########## @@ -69,11 +70,11 @@ struct TypeDescriptor { // explicit TypeDescriptor(PrimitiveType type) : TypeDescriptor(PrimitiveType type) : type(type), len(-1), precision(-1), scale(-1) { + // TODO, should not initialize default values, force initialization by parameters or external. Review Comment: 现在像是 ```c++ void SchemaScanner::_init_block(vectorized::Block* src_block) { const std::vector<SchemaScanner::ColumnDesc>& columns_desc(get_column_desc()); for (int i = 0; i < columns_desc.size(); ++i) { TypeDescriptor descriptor(columns_desc[i].type); auto data_type = vectorized::DataTypeFactory::instance().create_data_type(descriptor, true); src_block->insert(vectorized::ColumnWithTypeAndName(data_type->create_column(), data_type, columns_desc[i].name)); } } ``` 这种用法,是不是就是有问题的,对于有scale的类型,直接用PrimitiveType来构造之后使用,scale就没了? ########## be/src/runtime/types.h: ########## Review Comment: 这里面的`bool operator==`可能也有问题,可能最好删掉然后变成 `bool is(PrimitiveType rhs)` ########## be/src/vec/data_types/serde/data_type_date64_serde.cpp: ########## @@ -245,20 +252,28 @@ void DataTypeDate64SerDe::read_column_from_arrow(IColumn& column, const arrow::A } } else if (arrow_array->type()->id() == arrow::Type::STRING) { // to be compatible with old version, we use string type for date. - auto concrete_array = dynamic_cast<const arrow::StringArray*>(arrow_array); - for (size_t value_i = start; value_i < end; ++value_i) { - Int64 val = 0; + const auto* concrete_array = dynamic_cast<const arrow::StringArray*>(arrow_array); + for (auto value_i = start; value_i < end; ++value_i) { auto val_str = concrete_array->GetString(value_i); - ReadBuffer rb(val_str.data(), val_str.size()); - read_datetime_text_impl(val, rb, ctz); - col_data.emplace_back(val); + VecDateTimeValue v; + v.from_date_str(val_str.c_str(), val_str.length(), ctz); + if (is_date) { Review Comment: use `if constexpr` instead ########## be/test/vec/data_types/serde/data_type_serde_arrow_test.cpp: ########## @@ -300,19 +302,18 @@ void serialize_and_deserialize_arrow_test() { } break; case TYPE_DATETIMEV2: // uint64 { - // 2022-01-01 11:11:11.111 auto column_vector_datetimev2 = vectorized::ColumnVector<vectorized::UInt64>::create(); // auto& datetimev2_data = column_vector_datetimev2->get_data(); DateV2Value<DateTimeV2ValueType> value; - string date_literal = "2022-01-01 11:11:11.111"; - value.from_date_str(date_literal.c_str(), date_literal.size()); + string date_literal = "2022-01-01 11-11-11.111"; Review Comment: why change this? -- 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