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: dev-unsubscr...@datasketches.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@datasketches.apache.org For additional commands, e-mail: dev-h...@datasketches.apache.org