xiangfu0 commented on PR #19587: URL: https://github.com/apache/pinot/pull/19587#issuecomment-5975556765
@gortiz, I agree that the split-package and package-private shims should not ship. The [current implementation](https://github.com/apache/pinot/blob/a5eeef840787f19bf8bccdac369c113e2d016c20/pinot-common/src/main/java/org/apache/pinot/common/utils/RoaringBitmapUnion.java#L28-L69) puts both accumulators in `org.apache.pinot.common.utils`. Only private subclasses reach the protected `lazyor`/`repairAfterLazy` methods; there is no split package, `RoaringArray.mergeBulk` access, or use/extension of the deprecated `*Private` facades. `RoaringBitmapMerge` is removed, and intermediate-result merging uses ordinary `RoaringBitmap.or()`. This still depends on protected library behavior, so I am proposing it as an explicit interim implementation, not claiming the upstream concern is settled. The library-side proposal is [xiangfu0/RoaringBitmap#8](https://github.com/xiangfu0/RoaringBitmap/pull/8), currently in my personal fork; it has not been accepted or released upstream. It moves the owned incremental-union API and its container algorithms into RoaringBitmap rather than adding more low-level accessors. The class documents the migration: switch imports to the released library API, change `deserializeToUnion` to use its ownership entry point, then delete both Pinot wrappers and their tests. A test detects the proposed library class appearing on the classpath so the replacement is not silently forgotten. Pinot-specific aggregation, serialization boundaries, and index lifecycle stay here. The duplicate-density issue raised below is also fixed in a5eeef8, and current source CI is green. Could you re-review whether this explicitly temporary protected-subclass arrangement is acceptable, or whether you still prefer holding the Pinot PR until an upstream release? I am leaving that design decision open for maintainer agreement. -- 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]
