Gabriel39 commented on code in PR #69032:
URL: https://github.com/apache/doris/pull/69032#discussion_r4320668998
##########
be/src/core/data_type_serde/data_type_varbinary_serde.cpp:
##########
@@ -66,7 +77,7 @@ class VarbinaryParquetConsumer final : public
ParquetFixedValueConsumer,
}
private:
- ColumnVarbinary& _column;
Review Comment:
支持地理类型为什么要改varbinary？
##########
be/src/core/data_type_serde/data_type_varbinary_serde.cpp:
##########
@@ -172,19 +183,18 @@ Status
DataTypeVarbinarySerDe::write_column_to_arrow(const IColumn& column, cons
auto& builder =
assert_cast<arrow::FixedSizeBinaryBuilder&>(*array_builder);
const int byte_width =
static_cast<const
arrow::FixedSizeBinaryType&>(*array_builder->type()).byte_width();
- const auto& varbinary_column_data = assert_cast<const
ColumnVarbinary&>(column).get_data();
for (size_t i = start; i < end; ++i) {
if (null_map && (*null_map)[i]) {
RETURN_IF_ERROR(checkArrowStatus(builder.AppendNull(), column,
builder));
continue;
}
- const auto& string_view = varbinary_column_data[i];
- if (string_view.size() != byte_width) {
+ const auto string_view = column.get_data_at(i);
Review Comment:
```suggestion
const auto& string_view = column.get_data_at(i);
```
##########
be/src/core/data_type_serde/data_type_varbinary_serde.cpp:
##########
@@ -121,9 +132,9 @@ Status
DataTypeVarbinarySerDe::write_column_to_mysql_binary(const IColumn& colum
int64_t row_idx,
bool col_const,
const
FormatOptions& options) const {
auto col_index = index_check_const(row_idx, col_const);
- const auto& data = assert_cast<const
ColumnVarbinary&>(column).get_data()[col_index];
+ const auto data = column.get_data_at(col_index);
Review Comment:
```suggestion
const auto& data = column.get_data_at(col_index);
```
##########
be/src/core/data_type_serde/data_type_varbinary_serde.cpp:
##########
@@ -343,14 +353,14 @@ Status
DataTypeVarbinarySerDe::serialize_one_cell_to_json(const IColumn& column,
auto result = check_column_const_set_readability(column, row_num);
ColumnPtr ptr = result.first;
row_num = result.second;
- const auto& value = assert_cast<const
ColumnVarbinary&>(*ptr).get_data_at(row_num);
+ const auto value = ptr->get_data_at(row_num);
Review Comment:
这种逐行数据访问指针以及拷贝value的性能开销肯定是很大的
##########
be/src/format/parquet/parquet_column_convert.cpp:
##########
@@ -328,6 +342,13 @@ std::unique_ptr<PhysicalToLogicalConverter>
PhysicalToLogicalConverter::get_conv
physical_converter =
std::make_unique<UnsupportedConverter>(src_physical_type,
src_logical_type);
}
+ } else if (src_logical_primitive == TYPE_GEOMETRY || src_logical_primitive
== TYPE_GEOGRAPHY) {
+ if (src_physical_type == tparquet::Type::BYTE_ARRAY) {
Review Comment:
这个converter是什么时候用的？
##########
be/src/core/data_type_serde/data_type_varbinary_serde.cpp:
##########
@@ -135,15 +146,16 @@ Status
DataTypeVarbinarySerDe::write_column_to_arrow(const IColumn& column, cons
int64_t start, int64_t
end,
const cctz::time_zone&
ctz) const {
auto lambda_function = [&](auto& builder) -> Status {
- const auto& varbinary_column_data = assert_cast<const
ColumnVarbinary&>(column).get_data();
for (size_t i = start; i < end; ++i) {
if (null_map && (*null_map)[i]) {
RETURN_IF_ERROR(checkArrowStatus(builder.AppendNull(), column,
builder));
continue;
}
- const auto& string_view = varbinary_column_data[i];
-
RETURN_IF_ERROR(checkArrowStatus(builder.Append(string_view.data(),
string_view.size()),
- column, builder));
+ const auto string_view = column.get_data_at(i);
Review Comment:
```suggestion
const auto& string_view = column.get_data_at(i);
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
