leerho commented on issue #460: URL: https://github.com/apache/datasketches-cpp/issues/460#issuecomment-5925122265
Closing this issue: all three findings are now resolved in datasketches-cpp, and the fixes will ship in the upcoming 5.3.0 release. Thank you @nmahadevuni for the original report, and @jaideeppyne for the detailed differential analysis, which made each discrepancy easy to pin down. I verified each case against current C++ master by serializing the same sketches described above: ``` ┌──────────────────────────────────────┬─────────────┬──────────────────┬──────────────┐ │ Finding │ Java │ C++ master │ Fixed by │ ├──────────────────────────────────────┼─────────────┼──────────────────┼──────────────┤ │ 1. p bytes (2-item sketch) │ 00 00 00 00 │ 00 00 00 00 │ Java 9.0.0 ¹ │ ├──────────────────────────────────────┼─────────────┼──────────────────┼──────────────┤ │ 2. Single-item flag │ 3a │ 3a │ #526 ² │ ├──────────────────────────────────────┼─────────────┼──────────────────┼──────────────┤ │ 3. Empty seed hash │ 00 00 │ 00 00 │ #518 ³ │ ├──────────────────────────────────────┼─────────────┼──────────────────┼──────────────┤ │ 3b. A-not-B, empty B with other seed │ — │ no longer throws │ #517 ⁴ │ └──────────────────────────────────────┴─────────────┴──────────────────┴──────────────┘ ``` 1. C++ always wrote zero. Java changed to zero in 9.0.0. The bytes now match the original report exactly. 2. Set for both compact sketches and union results. 3. Also in the compressed (v4) form. 4. Also when B is an empty image written by Java. For example, the 2-item sketch from the original report now serializes on C++ master as: 02 03 03 00 00 1a cc 93 02 00 00 00 00 00 00 00 15 f9 7d cb bd 86 a1 05 c3 97 fc 12 81 70 9d 1e One remaining item is on the Java side: PreambleUtil.preambleToString() still derives the item count from the single-item flag. Since C++ now sets that flag, newly written C++ sketches display correctly in Java, but older images still show "CurrentCount: 0". That's a small diagnostic fix to track in datasketches-java. @jaideeppyne, if you'd still like to share your 20-case harness, it would be very welcome for our cross-language binary testing. -- 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]
