github-actions[bot] commented on code in PR #65715:
URL: https://github.com/apache/doris/pull/65715#discussion_r3636263767
##########
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)));
+ });
+ } else if (arrow_array->type_id() == arrow::Type::FIXED_SIZE_BINARY) {
+ const auto* concrete_array = assert_cast<const
arrow::FixedSizeBinaryArray*>(arrow_array);
+ if (config::enable_arrow_input_validation) {
+ check_arrow_array_range(*concrete_array, start, end);
+ check_arrow_fixed_width_buffer(*concrete_array,
Review Comment:
[P1] Reject unreadable FixedSizeBinary buffers before GetValue
The shared validation checks only that `buffers[1]` exists and reports
enough bytes. Arrow buffers can have a null data pointer with a positive
reported size, and non-CPU buffers return null from `data()`; such a width-1
array passes this check, then `GetValue()` feeds null to `insert_data()` and
crashes in `memcpy`. When required bytes are nonzero, require a CPU-readable
non-null data pointer before this loop and add a present-but-null buffer test.
--
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]
