Skip to content

GH-50251- Add GetSpan to ArrayData#50366

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

GH-50251- Add GetSpan to ArrayData#50366
dkp116 wants to merge 7 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.

Comment thread cpp/src/arrow/array/array_test.cc Outdated
Comment thread cpp/src/arrow/array/array_test.cc Outdated
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

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Hi @pitrou, thank you for reviewing, I have amended the tests and formatting

Comment thread cpp/src/arrow/array/array_test.cc
@github-actions github-actions Bot added awaiting committer review Awaiting committer review and removed awaiting review Awaiting review labels Jul 7, 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 @dkp116 , just some minor suggestions again

Comment thread cpp/src/arrow/array/array_test.cc
Comment thread cpp/src/arrow/array/array_test.cc
Comment thread cpp/src/arrow/array/array_test.cc Outdated
@pitrou

pitrou commented Jul 7, 2026

Copy link
Copy Markdown
Member

Also, please make sure your changes pass the CI lint step. You can do so locally using pre-commit run -a cpp.

@dkp116 dkp116 requested a review from pitrou July 7, 2026 12:32
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