github-actions[bot] commented on code in PR #65715:
URL: https://github.com/apache/doris/pull/65715#discussion_r3636263765
##########
be/src/core/data_type_serde/data_type_varbinary_serde.cpp:
##########
@@ -184,6 +185,98 @@ Status DataTypeVarbinarySerDe::write_column_to_arrow(const
IColumn& column, cons
return Status::OK();
}
+Status DataTypeVarbinarySerDe::read_column_from_arrow(IColumn& column,
+ const arrow::Array*
arrow_array,
+ int64_t start, int64_t
end,
+ const cctz::time_zone&
ctz) const {
+ auto& varbinary_column = assert_cast<ColumnVarbinary&>(column);
+ const auto read_binary = [&](const auto* concrete_array, const auto&
read_offset) -> Status {
+ const auto& buffer = concrete_array->value_data();
+ const size_t buffer_size = buffer ?
static_cast<size_t>(buffer->size()) : 0;
+ for (int64_t offset_i = start; offset_i < end; ++offset_i) {
+ if (concrete_array->IsNull(offset_i)) {
+ varbinary_column.insert_default();
+ continue;
+ }
+
+ const int64_t start_offset = read_offset(offset_i);
+ const int64_t end_offset = read_offset(offset_i + 1);
+ const int64_t length =
+ config::enable_arrow_input_validation
+ ? check_arrow_value_offsets(*concrete_array,
start_offset, end_offset,
+ buffer_size)
+ : end_offset - start_offset;
+ // Arrow may omit an empty values buffer. Do not form a pointer
from it for an empty
+ // value, but require a buffer before reading any non-empty value.
+ if (length == 0) {
+ varbinary_column.insert_data("", 0);
+ } else if (!buffer) {
+ return Status::InvalidArgument("Arrow {} values buffer is
missing",
+ concrete_array->type()->name());
+ } else {
+ varbinary_column.insert_data(
+ reinterpret_cast<const char*>(buffer->data() +
start_offset), length);
+ }
+ }
+ return Status::OK();
+ };
+
+ if (arrow_array->type_id() == arrow::Type::STRING ||
+ arrow_array->type_id() == arrow::Type::BINARY) {
+ const auto* concrete_array = assert_cast<const
arrow::BinaryArray*>(arrow_array);
+ if (config::enable_arrow_input_validation) {
+ check_arrow_array_range(*concrete_array, start, end);
+ check_arrow_binary_offsets_buffer(*concrete_array);
+ }
+ const auto& offsets = concrete_array->value_offsets();
+ if (!offsets) {
+ return Status::InvalidArgument("Arrow {} offsets buffer is
missing",
+ concrete_array->type()->name());
+ }
+ const auto* offsets_data = offsets->data();
+ return read_binary(concrete_array, [offsets_data](int64_t index) {
+ return static_cast<int64_t>(
+ unaligned_load<int32_t>(offsets_data + index *
sizeof(int32_t)));
Review Comment:
[P1] Add the Arrow slice offset to raw offset loads
`value_offsets()->data()` is the unsliced buffer base, but this lambda
indexes it with the slice-local row. With
`enable_arrow_input_validation=false`, a valid slice selecting row 1 from
`["a","bc"]` loads offsets `[0,1]` and returns `"a"` instead of `"bc"`; the
64-bit lambda below has the same bug, and `IsNull()` already uses the sliced
row. Add `concrete_array->offset()` to both unaligned-load indexes and cover
sliced 32-bit/64-bit inputs.
--
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]
