chore(deps): Revert - Upgrade jol-core to latest#148
Open
mblanco-denodo wants to merge 1 commit intoprestodb:masterfrom
Open
chore(deps): Revert - Upgrade jol-core to latest#148mblanco-denodo wants to merge 1 commit intoprestodb:masterfrom
mblanco-denodo wants to merge 1 commit intoprestodb:masterfrom
Conversation
This reverts the jol-core update that broke old jdk compatibility
Reviewer's guide (collapsed on small PRs)Reviewer's GuideReverts the previous jol-core upgrade and associated dependency and API changes to restore compatibility with older JDKs, including switching memory-size calculations back to int-based APIs and aligning parent/plugin versions. Class diagram for reverted HLL memory size API (class diagram)classDiagram
class HllInstance {
<<interface>>
+int getIndexBitLength()
+int estimatedInMemorySize()
+int estimatedSerializedSize()
+void insertHash(long hash)
+long cardinality()
}
class DenseHll {
-static int DENSE_INSTANCE_SIZE
-static int OVERFLOW_GROW_INCREMENT
-byte indexBitLength
-byte[] deltas
-int[] overflowBuckets
-byte[] overflowValues
+DenseHll(byte indexBitLength)
+void insertHash(long hash)
+long cardinality()
+int getIndexBitLength()
+int estimatedInMemorySize()
+int estimatedSerializedSize()
}
class SparseHll {
-static int SPARSE_INSTANCE_SIZE
-byte indexBitLength
-int maxHash
-int[] entries
+SparseHll(byte indexBitLength)
+void insertHash(long hash)
+long cardinality()
+int getIndexBitLength()
+int estimatedInMemorySize()
+int estimatedSerializedSize()
}
class HyperLogLog {
-static int INSTANCE_SIZE
-static int MAX_NUMBER_OF_BUCKETS
-HllInstance instance
+HyperLogLog(int indexBitLength)
+void insertHash(long hash)
+long cardinality()
+void eachBucket(BucketListener listener)
+int estimatedInMemorySize()
}
class QuantileDigest {
-static int MAX_BITS
-static int QUANTILE_DIGEST_SIZE
}
HllInstance <|.. DenseHll
HllInstance <|.. SparseHll
HyperLogLog o--> HllInstance
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The change from
longtointforestimatedInMemorySize()(and related constants) introduces a narrowing conversion and explicit cast inDenseHll; consider whether these size values could realistically exceedInteger.MAX_VALUEand, if so, keep the method/fields aslongto avoid silent truncation. - Reverting
jol-coreandairbaseversions may affect other code relying on APIs or behaviors from the newer versions; it may be safer to scope the revert only to the affected module or add a brief note in the commit message explaining why a full parent/dependency downgrade is necessary.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The change from `long` to `int` for `estimatedInMemorySize()` (and related constants) introduces a narrowing conversion and explicit cast in `DenseHll`; consider whether these size values could realistically exceed `Integer.MAX_VALUE` and, if so, keep the method/fields as `long` to avoid silent truncation.
- Reverting `jol-core` and `airbase` versions may affect other code relying on APIs or behaviors from the newer versions; it may be safer to scope the revert only to the affected module or add a brief note in the commit message explaining why a full parent/dependency downgrade is necessary.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
This was referenced Apr 9, 2026
imjalpreet
approved these changes
Apr 13, 2026
tdcmeehan
approved these changes
Apr 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



This reverts the jol-core update that broke old jdk compatibility: prestodb/presto#27382