peter-toth commented on PR #3597: URL: https://github.com/apache/parquet-java/pull/3597#issuecomment-4877687908
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. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
