GH-3596: Add `RowRanges.Builder` for incremental construction from selected row indices by peter-toth · Pull Request #3597 · apache/parquet-java

peter-toth · 2026-06-05T13:45:32Z

Rationale for this change

This PR is based on @mbutrovich's previous work.

Opening up APIs needed by a later materialization feature in Spark. External readers need to assemble a RowRanges incrementally from a stream of selected row indices (e.g. produced by a downstream filter or join) without having to know page boundaries ahead of time.

Since RowRanges is the type external readers must work with, this PR also promotes it out of the org.apache.parquet.internal.* package into a supported location.

What changes are included in this PR?

Adds a RowRanges.Builder for incremental construction. It accepts a strictly-increasing sequence of selected rows via addSelectedRow(long rowIndex) — or ranges via addSelectedRange(long from, long to), with addSelectedRow delegating to it — and coalesces consecutive/adjacent entries into Ranges. Out-of-order, overlapping, duplicate, negative, or from > to calls throw IllegalArgumentException. build() returns an independent snapshot, so the builder can be reused afterwards. Builder is final with a private constructor.
Moves RowRanges (with Range and Builder) from org.apache.parquet.internal.filter2.columnindex to org.apache.parquet.filter2.columnindex.
Preserves backward compatibility for the one released public API that referenced the old type: a deprecated org.apache.parquet.internal.filter2.columnindex.RowRanges subclass is kept, and ParquetFileReader.readFilteredRowGroup(int, RowRanges) gains a deprecated overload (taking the old internal type) that delegates to the new one. Both are marked for removal in 2.0.
Reimplements RowRanges.create(...) on top of the new Builder and keeps it on the deprecated internal.RowRanges (preserving the released internal.RowRanges.create(...) signature), so the public RowRanges no longer references the internal OffsetIndex; ColumnIndexFilter now builds via the public builder.
Other internal-only surfaces (RowRanges statics, Range, ColumnIndexFilter return types) are relocated without a bridge, as the internal package carries no compatibility guarantee.

Are these changes tested?

Yes. TestRowRanges covers single/multiple/coalesced ranges, the empty builder case, the out-of-order/duplicate/negative rejection paths, addSelectedRange coalescing and overlap/from > to rejection, the Long.MAX_VALUE edge case, and that build() returns a snapshot independent of the builder. Existing TestColumnIndexFilter and TestParquetFileReaderRowRanges continue to pass against the relocated type.

Are there any user-facing changes?

Yes. RowRanges (and Range/Builder) is now available at the supported package org.apache.parquet.filter2.columnindex. The old org.apache.parquet.internal.filter2.columnindex.RowRanges and ParquetFileReader.readFilteredRowGroup(int, <internal> RowRanges) remain as deprecated shims and will be removed in 2.0.

Closes #3596

…m selected row indices ### Rationale for this change Opening up APIs needed by a later materialization feature in Spark. External readers need to assemble a RowRanges incrementally from a stream of selected row indices (e.g. produced by a downstream filter or join) without having to know page boundaries ahead of time. ### What changes are included in this PR? Adds a Builder to RowRanges that takes a strictly-increasing sequence of selected row indices via addSelected(long) and coalesces consecutive indices into Range entries. Out-of-order or duplicate calls throw IllegalArgumentException. ### Are these changes tested? Yes. TestRowRanges covers single/multiple/coalesced ranges, the empty builder case, and the out-of-order/duplicate rejection paths. ### Are there any user-facing changes? No. Closes apache#3596 Co-authored-by: Matt Butrovich <mbutrovich@gmail.com>

peter-toth · 2026-06-29T14:39:14Z

@wgtmac, could you please take a look at this PR?

wgtmac

Codex review, focused only on API design.

wgtmac · 2026-06-30T08:25:13Z

Do you have any advice on this new API? @gszadovszky

…dRowGroup compatibility Recreate org.apache.parquet.internal.filter2.columnindex.RowRanges as a deprecated subclass of the relocated org.apache.parquet.filter2.columnindex.RowRanges so the released ParquetFileReader#readFilteredRowGroup(int, RowRanges) signature keeps linking. Add a deprecated readFilteredRowGroup overload taking the old internal type that delegates to the new one, and drop the japicmp exclusions for that method now that it is compatible. Other internal-only surfaces (RowRanges statics, Range, ColumnIndexFilter return types) remain relocated without a bridge.

ParquetFileReader lives in the downstream parquet-hadoop module, so a {@link} to it cannot be resolved from parquet-column's javadoc. Use a {@code} reference instead.

peter-toth · 2026-07-03T09:16:22Z

@wgtmac, @gszadovszky, please let me know how to proceed with this PR.

peter-toth · 2026-07-03T15:31:45Z

Thanks @wgtmac. I'm a bit confused, so let me lay out how things stand now:

How old and new relate. The real RowRanges implementation now lives at the new public location org.apache.parquet.filter2.columnindex.RowRanges. The old org.apache.parquet.internal.filter2.columnindex.RowRanges is kept as a thin deprecated subclass of it (Old extends New), purely so the released signatures keep linking.

The method you commented on. That overload is the deprecated one — readFilteredRowGroup(int, internal.RowRanges) is retained (and @Deprecated) as a shim that delegates to the new readFilteredRowGroup(int, public.RowRanges). So we already keep the original method; it's not going away until 2.0.

Why the excludes exist. They're not caused by readFilteredRowGroup — that signature is preserved, so japicmp is happy with it. The excludes are for the internal package itself: making Old an empty subclass moved its members (statics, Range) into New, and ColumnIndexFilter.calculateRowRanges now returns the public RowRanges. Since internal.* carries no backward-compat guarantee, those are intentional and that's what the three excludes cover.

On dropping the excludes. Reversing the inheritance (New extends Old) would indeed let us avoid the excludes now — but I think it just moves the cost to 2.0: when we delete the old parent, New loses its superclass and all the inherited members, so we'd have to pull them up into New and add fresh excludes for the class we actually keep. The current direction makes Old a deletable leaf, so in 2.0 these three excludes just get removed along with it — no new ones.

The other option is to fully separate New from Old (no shared type), but then ColumnIndexFilter and the range-algebra helpers would have to be duplicated to produce the new type (or we add an Old → New conversion layer), which felt worse than three internal-package excludes.

Do you have a concrete approach in mind? I'm happy to explore it.

peter-toth · 2026-07-08T07:55:06Z

@wgtmac , any suggestion on the above? Do you think the current implementation is fine or shall try to fully separate old and new RowRanges?

wgtmac

Sorry for the delay. Now it overall looks good. I just have two more comments. Let me know what you think. Thanks!

Check rowIndex <= runEnd before evaluating runEnd + 1, so the strictly-increasing guard keeps working when runEnd is Long.MAX_VALUE (runEnd + 1 would otherwise overflow to Long.MIN_VALUE and let a follow-up index start a bogus new run).

… move it to internal.RowRanges Adds a range-level Builder.addSelectedRange(from, to) (addSelectedRow now delegates to it), so RowRanges can be built from page ranges through the public API. create() is reimplemented on top of the builder and moved to the deprecated internal.RowRanges shim, where the released 1.17 internal.RowRanges.create(...) signature lived. The public RowRanges no longer references the internal OffsetIndex, and ColumnIndexFilter now builds via the public builder rather than a deprecated method. Behavior note: the builder rejects overlapping ranges instead of silently unioning them (as the old add() did). Offset-index pages never overlap, so the filter path is unaffected; TestRowRanges#testCreate is updated to feed realistic adjacent (non-overlapping) page ranges.

wgtmac

Thanks @peter-toth, now it looks good to me. It would be good to wait for @gszadovszky to take a look in case I missed anything.

gszadovszky

It looks good to me, too.
Thank you @peter-toth and @wgtmac!

peter-toth · 2026-07-14T07:21:26Z

Thank you @wgtmac and @gszadovszky for the review!

peter-toth mentioned this pull request Jun 5, 2026

Add a RowRanges.Builder for incremental construction from selected row indices #3596

Closed

peter-toth force-pushed the GH-3596-row-ranges-builder branch from e11f3d9 to 58857df Compare June 5, 2026 13:56

peter-toth mentioned this pull request Jun 8, 2026

GH-3598: Expose getRowRanges(int) #3599

Merged

wgtmac reviewed Jun 30, 2026

View reviewed changes

Peter Toth added 6 commits June 30, 2026 14:42

address review comments

17cce69

move RowRanges

8d8e2dd

Merge branch 'master' into apacheGH-3596-row-ranges-builder

bdeaedc

fix compile

5f53c99

apacheGH-3596: Fix javadoc reference in deprecated RowRanges shim

8f17e2a

ParquetFileReader lives in the downstream parquet-hadoop module, so a {@link} to it cannot be resolved from parquet-column's javadoc. Use a {@code} reference instead.

wgtmac reviewed Jul 3, 2026

View reviewed changes

Comment thread parquet-hadoop/src/main/java/org/apache/parquet/hadoop/ParquetFileReader.java

wgtmac reviewed Jul 11, 2026

View reviewed changes

Comment thread parquet-column/src/main/java/org/apache/parquet/filter2/columnindex/RowRanges.java Outdated

Comment thread parquet-column/src/main/java/org/apache/parquet/filter2/columnindex/RowRanges.java Outdated

Peter Toth added 2 commits July 11, 2026 09:34

wgtmac approved these changes Jul 11, 2026

View reviewed changes

gszadovszky approved these changes Jul 13, 2026

View reviewed changes

wgtmac merged commit 06ed267 into apache:master Jul 14, 2026
5 checks passed

Uh oh!

Conversation

peter-toth commented Jun 5, 2026 • edited 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?

Uh oh!

peter-toth commented Jun 29, 2026

Uh oh!

wgtmac left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wgtmac commented Jun 30, 2026

Uh oh!

peter-toth commented Jul 3, 2026

Uh oh!

Uh oh!

peter-toth commented Jul 3, 2026

Uh oh!

peter-toth commented Jul 8, 2026

Uh oh!

wgtmac left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

wgtmac left a comment

Choose a reason for hiding this comment

Uh oh!

gszadovszky left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

peter-toth commented Jul 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

peter-toth commented Jun 5, 2026 •

edited

Loading

wgtmac left a comment •

edited

Loading