Skip to content

GH-50251- Add GetSpan to ArrayData#50366

Open
dkp116 wants to merge 4 commits into
apache:mainfrom
dkp116:GH-50251
Open

GH-50251- Add GetSpan to ArrayData#50366
dkp116 wants to merge 4 commits into
apache:mainfrom
dkp116:GH-50251

Conversation

@dkp116

@dkp116 dkp116 commented Jul 4, 2026

Copy link
Copy Markdown

Rationale for this change

( #50251 ) Enhancement for ArrayData

What changes are included in this PR?

Added GetSpan(int i, int64_t length) for read-only typed access to ArrayData.
Added GetMutableSpan(int i, int64_t length) for writable typed access to ArrayData.

Are these changes tested?

New unit tests were added in array_test.cc:

GetSpanRespectsOffset
GetMutableSpanRespectsOffset

The tests verify:

correct slicing with offset
correct span length
correctness of data read through spans
correctness of in-place mutation through GetMutableSpan

@dkp116 dkp116 requested a review from pitrou as a code owner July 4, 2026 22:00
@dkp116 dkp116 changed the title GH-50251 GH-50310 Jul 4, 2026
@github-actions

github-actions Bot commented Jul 4, 2026

Copy link
Copy Markdown

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

@dkp116 dkp116 changed the title GH-50310 GH-50251 Jul 4, 2026
@dkp116 dkp116 changed the title GH-50251 GH-50251- Add GetSpan to ArrayData Jul 4, 2026

@pitrou pitrou left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for doing this @dkp116 . Here are two comments, otherwise this looks ok to me.

TEST_F(TestArray, GetSpanRespectsOffset) {
auto data_buffer = Buffer::FromString("123456789abcdef0");
auto data = ArrayData::Make(uint8(), 3, {nullptr, data_buffer}, 0, 1);
auto span = data->GetSpan<uint8_t>(1, 3);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you test with a non-byte sized data type?

Comment on lines +1156 to +1157
auto data = ArrayData::Make(uint8(), 3, buffers, 0, 1);
auto span = data->template GetMutableSpan<uint8_t>(1, 3);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment here

EXPECT_EQ(span[0], '2');
EXPECT_EQ(span[1], '3');
EXPECT_EQ(span[2], '4');
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: please leave an empty line after each test function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants