github-actions[bot] commented on code in PR #64829:
URL: https://github.com/apache/doris/pull/64829#discussion_r3474586955
##########
be/src/core/data_type_serde/data_type_string_serde.cpp:
##########
@@ -281,11 +281,10 @@ Status
DataTypeStringSerDeBase<ColumnType>::read_column_from_arrow(
} else if (arrow_array->type_id() == arrow::Type::FIXED_SIZE_BINARY) {
const auto* concrete_array = dynamic_cast<const
arrow::FixedSizeBinaryArray*>(arrow_array);
uint32_t width = concrete_array->byte_width();
- const auto* array_data = concrete_array->GetValue(start);
- for (size_t offset_i = 0; offset_i < end - start; ++offset_i) {
+ for (auto offset_i = start; offset_i < end; ++offset_i) {
Review Comment:
This fixes the string serde branch, but the parallel JSONB Arrow reader
still has the same mixed-index bug. In
`DataTypeJsonbSerDe::read_column_from_arrow`, the `FIXED_SIZE_BINARY` case
still does `GetValue(start)`, loops `offset_i` from `0`, and checks
`IsNull(offset_i)`. Nested array/map/struct readers pass nonzero child ranges
into nested serdes, so an `ARRAY<JSONB>` or `MAP<..., JSONB>` backed by
fixed-size binary Arrow values can still apply row 0's null bit while reading
row `start`'s bytes. Please update the JSONB fixed-size-binary branch to use
the same absolute `[start, end)` indexing as this string branch, and add the
corresponding JSONB/nested coverage if possible.
--
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]
