Skip to content

Conversation

@poshul
Copy link

@poshul poshul commented Dec 23, 2025

Rationale for this change

Based on @wgtmac 's #45234 This attempts to address the comments in that PR

What changes are included in this PR?

add an experimental row_selection API

Are these changes tested?

Yes

Are there any user-facing changes?

Yes

@poshul poshul requested a review from wgtmac as a code owner December 23, 2025 14:41
@github-actions
Copy link

Thanks for opening a pull request!

If this is not a minor PR. Could you open an issue for this pull request on GitHub? https://github.com/apache/arrow/issues/new/choose

Opening GitHub issues ahead of time contributes to the Openness of the Apache Arrow project.

Then could you also rename the pull request title in the following format?

GH-${GITHUB_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}

or

MINOR: [${COMPONENT}] ${SUMMARY}

See also:

@poshul poshul changed the title Row selection GH-45284: [Parquet][C++] Proposed RowRanges API Jan 5, 2026
@poshul
Copy link
Author

poshul commented Jan 5, 2026

@emkornfield any thoughts on this as a proposed first step?

@github-actions
Copy link

github-actions bot commented Jan 5, 2026

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

/// \brief EXPERIMENTAL: Validate the row ranges.
/// \throws ParquetException if the row ranges are not in ascending order or
/// overlapped.
void Validate() const;
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems like validation should happen on construction?

Copy link
Author

Choose a reason for hiding this comment

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

That probably makes sense. I've moved it so that it gets called before any of the functions the create RowSelections return.

@emkornfield
Copy link
Contributor

Took a look at the top-level public API, looks reasonable to me. If you need a full review please ping me.

Apologies for the delay.

@poshul
Copy link
Author

poshul commented Feb 2, 2026

Took a look at the top-level public API, looks reasonable to me. If you need a full review please ping me.

Apologies for the delay.

No worries for the delay, January is just a difficult month. I would appreciate a full review if you have the time for it.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants