github-actions[bot] commented on code in PR #66819:
URL: https://github.com/apache/doris/pull/66819#discussion_r3793802599
##########
be/src/format/arrow/arrow_array_normalizer.cpp:
##########
@@ -48,6 +51,55 @@ std::shared_ptr<arrow::DataType> target_type_for(const
arrow::DataType& type) {
}
}
+template <typename ViewArray, typename ListBuilder, typename ListArray>
+arrow::Result<std::shared_ptr<arrow::Array>> canonicalize_list_view(const
ViewArray& source,
+
arrow::MemoryPool* pool) {
+ // Imported C stream arrays are not guaranteed to have validated ranges;
copying an invalid
+ // range before this check could read beyond the child array.
+ auto validation = source.ValidateFull();
+ if (!validation.ok()) {
+ return validation;
+ }
+
+ auto child_builder_result = arrow::MakeBuilder(source.value_type(), pool);
+ if (!child_builder_result.ok()) {
+ return child_builder_result.status();
+ }
+ std::shared_ptr<arrow::ArrayBuilder>
child_builder(child_builder_result.MoveValueUnsafe());
+ ListBuilder builder(pool, child_builder);
+ auto reserve_status = builder.Reserve(source.length());
+ if (!reserve_status.ok()) {
+ return reserve_status;
+ }
+
+ arrow::ArraySpan values(*source.values()->data());
+ for (int64_t i = 0; i < source.length(); ++i) {
+ if (source.IsNull(i)) {
+ auto append_status = builder.AppendNull();
+ if (!append_status.ok()) {
+ return append_status;
+ }
+ continue;
+ }
+ auto append_status = builder.Append();
+ if (!append_status.ok()) {
+ return append_status;
+ }
+ append_status = child_builder->AppendArraySlice(values,
source.value_offset(i),
Review Comment:
[P1] Check aggregate lengths before copying into builders
A valid `LargeListView<Null>` can reach signed overflow here without
allocating a huge buffer: use a Null child of length `INT64_MAX` and two shared
ranges of length `INT64_MAX - 1`. `ValidateFull()` accepts both ranges
independently, but Arrow 24's `NullBuilder::AppendArraySlice` performs
unchecked `length_ += length`, so the second append overflows before
`LargeListBuilder` checks capacity. Checking only this source's top-level total
is not enough either: an outer `large_list_view<large_list_view<null>>` can
have total 2 while the nested builder overflows on the same ranges during this
call. Please compute checked non-null logical totals before any append at every
copied view level, or reject nested view children before building, and add a
no-allocation regression test.
##########
be/src/format/arrow/arrow_array_normalizer.cpp:
##########
@@ -48,6 +51,55 @@ std::shared_ptr<arrow::DataType> target_type_for(const
arrow::DataType& type) {
}
}
+template <typename ViewArray, typename ListBuilder, typename ListArray>
+arrow::Result<std::shared_ptr<arrow::Array>> canonicalize_list_view(const
ViewArray& source,
+
arrow::MemoryPool* pool) {
+ // Imported C stream arrays are not guaranteed to have validated ranges;
copying an invalid
+ // range before this check could read beyond the child array.
+ auto validation = source.ValidateFull();
+ if (!validation.ok()) {
+ return validation;
+ }
+
+ auto child_builder_result = arrow::MakeBuilder(source.value_type(), pool);
+ if (!child_builder_result.ok()) {
+ return child_builder_result.status();
+ }
+ std::shared_ptr<arrow::ArrayBuilder>
child_builder(child_builder_result.MoveValueUnsafe());
+ ListBuilder builder(pool, child_builder);
+ auto reserve_status = builder.Reserve(source.length());
Review Comment:
[P1] Reserve the expanded child before the append loop
Only the parent slots are reserved, so the child grows geometrically while
shared ListView ranges are expanded. For example, 1,025 rows viewing the same
100,000 Int32 values produce a roughly 391 MiB value buffer, but Arrow's final
doubling reallocates it from about 391 MiB to 781 MiB; Doris charges the new
allocation before releasing the old one, transiently accounting roughly 1.17
GiB for that buffer alone. An exact reserve keeps the canonical and Doris value
buffers near 782 MiB combined, leaving roughly 391 MiB less required headroom
(other common query allocations aside). After validation, please compute a
checked non-null logical total and reserve it on `child_builder` before this
loop.
--
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]
