GH-52021: [Ruby] Add support for sliced arrays in MemoryView by gitedmond · Pull Request #52286 · apache/arrow

gitedmond · 2026-10-07T19:57:39Z

Rationale for this change

MemoryView export of Arrow::PrimitiveArray didn't support arrays with non-zero offset (sliced arrays). For example, exporting Arrow::Int32Array.new([0, 1, 2, 3]).slice(1, 3) returned wrong values because the data pointer was advanced by offset bytes instead of offset * byte width bytes.

What changes are included in this PR?

Use offset * bit width / 8 as the byte offset of the data pointer. This supports sliced fixed-width arrays.
Support sliced boolean arrays only when the offset is byte aligned. MemoryView can't represent a bit offset.
Implement available_p for Arrow::PrimitiveArray. It returns false for arrays that can't be represented by MemoryView: unsupported types and boolean arrays with not byte aligned offset.

Are these changes tested?

Ran the complete test-memory-view.rb file through the native Ruby/Fiddle interface using Ruby 3.2.3, Fiddle 1.1.8, and Arrow C++/GLib 25.0.1.

The original MemoryView implementation in the 25.0.1 release is identical to the current checkout. The extension was rebuilt with this PR's updated memory-view.cpp.

Revised complete test file: 29 tests, 31 assertions, 0 failures, 0 errors, 0 omissions.
Both new Boolean cases fail against the previous PR implementation and pass after the revision.
git diff --check passes.

The full repository test suite and a complete build of the 26.0.0-SNAPSHOT dependencies were not run.

Are there any user-facing changes?

Sliced fixed-width arrays and byte-aligned sliced boolean arrays can be exported by MemoryView.
rb_memory_view_available_p() returns false for unsupported arrays.

Was AI used for this PR?

AI assisted with the investigation, implementation, regression tests, validation, and PR description.

PR code and description written by:

Human
AI

Reviewed before submission by:

Human
AI
Not reviewed

GitHub Issue: [Ruby] Wrong data pointer in MemoryView of a sliced array #52021

github-actions · 2026-10-07T19:58:14Z

⚠️ GitHub issue #52021 has been automatically assigned in GitHub to PR creator.

kou

Reviewed before submission by:

Human

AI

Not reviewed

You haven't reviewed this, right? If so, can I open a separated PR that fixes my review comments?

See also: https://arrow.apache.org/docs/dev/developers/overview.html#ai-generated-code

BTW, sliced arrays just weren't supported before. I want to change the PR title to something like "Add support for sliced arrays in MemoryView".

kou · 2026-10-08T08:08:02Z

          std::static_pointer_cast<const arrow::FixedWidthType>(array.type());
        view_->item_size = type->bit_width() / 8;
+        const auto byte_offset =
+          array_data->offset * std::max<int64_t>(view_->item_size, 1);


Is this std::max<int64_t>() for non byte aligned (offset % 8 != 0) boolean array? We should reject the case because we can't represent the case by MemoryView.

Removed std::max. The byte offset now uses the type’s bit width, and Boolean slices with offset % 8 != 0 are rejected. Added separate tests for byte-aligned export and non-byte-aligned rejection.

kou · 2026-10-08T08:18:00Z

+  [
+    [Arrow::Int8Array, "c"],
+    [Arrow::UInt8Array, "C"],
+    [Arrow::Int16Array, "s"],
+    [Arrow::UInt16Array, "S"],
+    [Arrow::Int32Array, "l"],
+    [Arrow::UInt32Array, "L"],
+    [Arrow::Int64Array, "q"],
+    [Arrow::UInt64Array, "Q"],
+    [Arrow::FloatArray, "f"],
+    [Arrow::DoubleArray, "d"],
+  ].each do |array_class, format|
+    test("#{array_class.name}: sliced") do


We don't want to define tests dynamically. Could you follow existing style?

We can use the following style here:

sub_test_case("Int8Array") do test("offset: 0") do # Existing test("Int8Array") content end test("offset: non-0") do # This PR case end end sub_test_case("Int8Array") do # ... end # ...

Replaced the dynamically generated tests with explicit sub_test_case groups containing offset: 0 and offset: non-0 tests, following your suggested style.

kou · 2026-10-08T08:19:45Z

+      slices.each do |sliced_array, sliced_values|
+        Fiddle::MemoryView.export(sliced_array) do |memory_view|
+          assert_equal([
+                         format,
+                         item_size,
+                         item_size * sliced_values.size,
+                         sliced_values.pack("#{format}*"),
+                       ],
+                       [
+                         memory_view.format,
+                         memory_view.item_size,
+                         memory_view.byte_size,
+                         memory_view.to_s,
+                       ])
+        end


We don't want to use each {assert_equal(...)} for easy to debug. If we use the style, we can't run only the target case. We need to run all cases. And all cases aren't executed if one case is failed.

Removed the assertion loop. Each retained slice case is now a separately named test that can run independently.

kou · 2026-10-08T08:21:14Z

+        [array.slice(1, 3), values.slice(1, 3)],
+        [array.slice(4, 3), values.slice(4, 3)],
+        [array.slice(1, 6).slice(2, 3), values.slice(3, 3)],
+        [array.slice(4, 0), []],
+        [array.slice(8, 0), []],


Do we need all cases? We want to remove needless cases as much as possible to focus on only important parts.

Trimmed the repeated offsets, nested slices, empty slices, and duplicate numeric-type coverage. Kept one sliced case per numeric byte width, one fixed-size binary case, and the Boolean alignment cases.

github-actions · 2026-10-08T12:21:36Z

⚠️ GitHub issue #52021 has been automatically assigned in GitHub to PR creator.

gitedmond · 2026-10-08T12:23:52Z

Updated the title to "Add support for sliced arrays in MemoryView" and addressed the four inline comments in 0638ba3. Rebuilt the extension and ran the complete MemoryView test file: all 29 tests pass. The two new Boolean cases fail against the previous PR implementation and pass with the revision.

kou · 2026-10-08T21:47:56Z

I've pushed some commits to use rb_memory_view_available_p() for unsupported arrays and cleanup. I've also update the PR description to match the latest implementation.

Could you review them before we merge this?

gitedmond · 2026-10-09T00:33:28Z

@kou lgtm!

kou · 2026-10-09T01:01:28Z

Thanks. Merged.

apacheGH-52021: [Ruby] Fix MemoryView data offsets for sliced arrays

14a0ab6

gitedmond requested a review from kou as a code owner October 7, 2026 19:57

github-actions Bot added Component: Ruby awaiting review Awaiting review labels Oct 7, 2026

gitedmond mentioned this pull request Oct 7, 2026

[Ruby] Wrong data pointer in MemoryView of a sliced array #52021

Closed

kou reviewed Oct 8, 2026

View reviewed changes

github-actions Bot added awaiting changes Awaiting changes and removed awaiting review Awaiting review labels Oct 8, 2026

apacheGH-52021: [Ruby] Address MemoryView review feedback

0638ba3

github-actions Bot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Oct 8, 2026

gitedmond changed the title ~~GH-52021: [Ruby] Fix MemoryView data offsets for sliced arrays~~ GH-52021: [Ruby] Add support for sliced arrays in MemoryView Oct 8, 2026

kou added 2 commits October 9, 2026 06:39

Implement available_p() to reject unsupported arrays

63fa438

Extract common code

23902bd

kou merged commit c68e0aa into apache:main Oct 9, 2026
34 checks passed

kou removed the awaiting change review Awaiting change review label Oct 9, 2026

Conversation

gitedmond commented Oct 7, 2026 • edited by kou Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

Was AI used for this PR?

Uh oh!

github-actions Bot commented Oct 7, 2026

Uh oh!

kou left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Oct 8, 2026

Uh oh!

gitedmond commented Oct 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kou commented Oct 8, 2026

Uh oh!

gitedmond commented Oct 9, 2026

Uh oh!

Uh oh!

kou commented Oct 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

gitedmond commented Oct 7, 2026 •

edited by kou

Loading

gitedmond commented Oct 8, 2026 •

edited

Loading