jmalkin commented on code in PR #685:
URL: https://github.com/apache/datasketches-java/pull/685#discussion_r2408965856
##########
src/main/java/org/apache/datasketches/theta/CompactSketch.java:
##########
@@ -87,119 +81,85 @@ public static CompactSketch heapify(final MemorySegment
srcSeg) {
* <p>The resulting sketch will not retain any link to the source
MemorySegment and all of its data will be
* copied to the heap CompactSketch.</p>
*
- * <p>This method checks if the given expectedSeed was used to create the
source MemorySegment image.
- * However, SerialVersion 1 sketch images cannot be checked as they don't
have a seedHash field,
- * so the resulting heapified CompactSketch will be given the hash of the
expectedSeed.</p>
+ * <p>This method checks if the given expectedSeed was used to create the
source MemorySegment image.</p>
*
* @param srcSeg an image of a CompactSketch that was created using the
given expectedSeed.
* @param expectedSeed the seed used to validate the given MemorySegment
image.
* <a href="{@docRoot}/resources/dictionary.html#seed">See Update Hash
Seed</a>.
* @return a CompactSketch on the heap.
*/
public static CompactSketch heapify(final MemorySegment srcSeg, final long
expectedSeed) {
- return heapify(srcSeg, expectedSeed, true);
- }
-
- private static CompactSketch heapify(final MemorySegment srcSeg, final long
seed, final boolean enforceSeed) {
final int serVer = extractSerVer(srcSeg);
final int familyID = extractFamilyID(srcSeg);
final Family family = idToFamily(familyID);
if (family != Family.COMPACT) {
throw new SketchesArgumentException("Corrupted: " + family + " is not
Compact!");
}
if (serVer == 4) {
- return heapifyV4(srcSeg, seed, enforceSeed);
+ return heapifyV4(srcSeg, expectedSeed);
}
if (serVer == 3) {
final int flags = extractFlags(srcSeg);
final boolean srcOrdered = (flags & ORDERED_FLAG_MASK) != 0;
final boolean empty = (flags & EMPTY_FLAG_MASK) != 0;
- if (enforceSeed && !empty) { PreambleUtil.checkSegmentSeedHash(srcSeg,
seed); }
+ if (!empty) { PreambleUtil.checkSegmentSeedHash(srcSeg, expectedSeed); }
return CompactOperations.segmentToCompact(srcSeg, srcOrdered, null);
}
- //not SerVer 3, assume compact stored form
- final short seedHash = Util.computeSeedHash(seed);
- if (serVer == 1) {
Review Comment:
@leerho proposed this on the mailing list and received supportive comments.
Versions 1 and 2 predate the open source release, so only Yahoo might have
such sketches. And since they can be read and converted to v3 using older
library versions, it seems like a reasonable time to clean up this legacy code.
--
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]
