refactor: use raw view access in do_append_val_inner and consolidate duplicated logic (#22907)

## Which issue does this PR close?

Follow-up to #21794, addressing review feedback from @alamb and
@Dandandan.

## Rationale for this change

In the review of #21794, several optimizations were suggested:

- **@alamb**
([comment](#21794 (comment))):
"as a possible future optimization, we could use `get_unchecked` here if
it makes any difference" — referring to the slow-path `arr.views()[row]`
access.
- **@alamb**
([comment](#21794 (comment))):
"from here on down I think this is basically the same as
append_val_inner -- if there are any differences perhaps we can fold it
into append_val_inner and avoid the copy"
- **@Dandandan**
([comment](#21794 (comment))):
"In principle we can make this faster as well - `extend` + reuse input
view (instead of make_view) + avoid `array.value(row)`"

## What changes are included in this PR?

**1. Refactored `do_append_val_inner` to use raw view access**

Replaced `array.value(row)` + `make_view()` with raw view access via
`get_unchecked(row)`:
- **Inline (len <= 12):** push the u128 view as-is — no decode/re-encode
round-trip
- **Non-inline (len > 12):** parse via `ByteView::from(view)`, copy
buffer data, reuse source prefix directly (avoids re-reading first 4
bytes)

**2. Simplified the vectorized slow path**

Replaced the duplicated 28-line loop body in `vectorized_append_inner`
with `try_reserve` + a loop calling `do_append_val_inner`, eliminating
code duplication.

**3. Removed unused `make_view` import**

### Safety notes

- **`get_unchecked` usage**: Consistent with `do_equal_to_inner` (same
file) and `PrimitiveGroupValueBuilder` in `primitive.rs`, both of which
use the same pattern. All callers derive row indices from enumeration
over the input array length, guaranteeing validity.
- **Buffer access safety**: When `data_buffers()` is empty, all views
must have len <= 12 (Arrow invariant), so the non-inline branch is never
entered.

## Are these changes tested?

Covered by 6 existing unit tests in the `bytes_view` module plus 3
integration tests in the `multi_group_by` module. All 111 tests in the
aggregates suite pass.

## Are there any user-facing changes?

No. This is an internal refactor with no API changes.

Author: EeshanBembi

datafusion/physical-plan/src/aggregates/group_values/multi_group_by/bytes_view.rs

@@ -21,7 +21,6 @@ use crate::aggregates::group_values::multi_group_by::{
 use crate::aggregates::group_values::null_builder::MaybeNullBufferBuilder;
 use arrow::array::{
     Array, ArrayRef, AsArray, BooleanBufferBuilder, ByteView, GenericByteViewArray,
-    make_view,
 };
 use arrow::buffer::{Buffer, ScalarBuffer};
 use arrow::datatypes::ByteViewType;
@@ -176,43 +175,17 @@ impl<B: ByteViewType> ByteViewGroupValueBuilder<B> {
                     // copy them directly instead of going through value() → make_view().
                     self.views.extend(rows.iter().map(|&row| arr.views()[row]));
                 } else {
-                    // Slow path: some strings are non-inline (>12 bytes).
-                    // Read views directly to avoid array.value(row) overhead and
-                    // reuse the source view's prefix instead of recomputing it via make_view.
+                    // Slow path: some strings may be non-inline (>12 bytes).
+                    // Pre-reserve and delegate to do_append_val_inner which
+                    // reads raw views directly and reuses source prefixes.
                     self.views.try_reserve(rows.len()).map_err(|e| {
                         datafusion_common::exec_datafusion_err!(
                             "failed to reserve {0} views: {e}",
                             rows.len()
                         )
                     })?;
                     for &row in rows {
-                        let view = arr.views()[row];
-                        let len = view as u32;
-                        if len <= 12 {
-                            // This row happens to be inline; copy view directly.
-                            self.views.push(view);
-                        } else {
-                            let src = ByteView::from(view);
-                            // ensure_in_progress_big_enough must be called before computing
-                            // new_buffer_index / new_offset — it may flush in_progress to completed.
-                            self.ensure_in_progress_big_enough(len as usize);
-                            let new_buffer_index = self.completed.len() as u32;
-                            let new_offset = self.in_progress.len() as u32;
-                            let src_buf = &arr.data_buffers()[src.buffer_index as usize];
-                            self.in_progress.extend_from_slice(
-                                &src_buf[src.offset as usize
-                                    ..(src.offset + src.length) as usize],
-                            );
-                            // Reuse prefix from the source view — avoids re-reading first 4 bytes.
-                            let new_view = ByteView {
-                                length: src.length,
-                                prefix: src.prefix,
-                                buffer_index: new_buffer_index,
-                                offset: new_offset,
-                            }
-                            .as_u128();
-                            self.views.push(new_view);
-                        }
+                        self.do_append_val_inner(arr, row);
                     }
                 }
             }
@@ -230,25 +203,33 @@ impl<B: ByteViewType> ByteViewGroupValueBuilder<B> {
     where
         B: ByteViewType,
     {
-        let value: &[u8] = array.value(row).as_ref();
+        // SAFETY: the caller ensures `row` is valid
+        let view = unsafe { *array.views().get_unchecked(row) };
+        let len = view as u32;
 
-        let value_len = value.len();
-        let view = if value_len <= 12 {
-            make_view(value, 0, 0)
+        if len <= 12 {
+            // Inline value: the view is already self-contained, push as-is.
+            self.views.push(view);
         } else {
-            // Ensure big enough block to hold the value firstly
-            self.ensure_in_progress_big_enough(value_len);
-
-            // Append value
-            let buffer_index = self.completed.len();
-            let offset = self.in_progress.len();
-            self.in_progress.extend_from_slice(value);
-
-            make_view(value, buffer_index as u32, offset as u32)
-        };
-
-        // Append view
-        self.views.push(view);
+            // Non-inline value: copy the buffer data and construct a new view
+            // that points into our own buffers, reusing the source prefix.
+            let src = ByteView::from(view);
+            self.ensure_in_progress_big_enough(len as usize);
+            let new_buffer_index = self.completed.len() as u32;
+            let new_offset = self.in_progress.len() as u32;
+            let src_buf = &array.data_buffers()[src.buffer_index as usize];
+            self.in_progress.extend_from_slice(
+                &src_buf[src.offset as usize..(src.offset + src.length) as usize],
+            );
+            let new_view = ByteView {
+                length: src.length,
+                prefix: src.prefix,
+                buffer_index: new_buffer_index,
+                offset: new_offset,
+            }
+            .as_u128();
+            self.views.push(new_view);
+        }
     }
 
     fn ensure_in_progress_big_enough(&mut self, value_len: usize) {