proost commented on code in PR #790:
URL: https://github.com/apache/datasketches-java/pull/790#discussion_r4443756532
##########
src/test/java/org/apache/datasketches/hll/HllSketchCrossLanguageTest.java:
##########
@@ -136,6 +144,95 @@ private static void deserializeHll(final GroupLanguage
lang) {
}
}
+ /**
+ * The fixtures above are update sketches only, serialized compact, at a
single lgK. That
+ * leaves two gaps a cross-language reader can fall through, both of which
produced real bugs
+ * in 2026-09: the deferred KxQ rebuild is reachable only through a union
gadget, and the
+ * flags-byte bit 32 collision is only visible in the flags byte itself.
These fixtures close
+ * both. One lgK is deliberate; the defect classes are not lgK-dependent.
+ */
+ @Test(groups = {GENERATE_JAVA_FILES}, priority = 0)
+ public void generateUnionBinariesForCompatibilityTesting() throws
IOException {
+ for (final TgtHllType type: new TgtHllType[] {HLL_4, HLL_6, HLL_8}) {
+ final HllUnion union = mixedModeUnion();
+ final String tag = tagOf(type);
+ putBytesToJavaPath(tag + "_union_java.sk",
union.getResult(type).toCompactByteArray());
+ putBytesToJavaPath(tag + "_unionupd_java.sk",
union.getResult(type).toUpdatableByteArray());
+ }
+ }
+
+ @Test(groups = {CHECK_JAVA_FILES}, priority = 1)
+ public void checkUnionJava() { deserializeUnionResults(GroupLanguage.JAVA); }
+
+ @Test(groups = {CHECK_CPP_FILES})
+ public void checkUnionCpp() { deserializeUnionResults(GroupLanguage.CPP); }
+
+ @Test(groups = {CHECK_GO_FILES})
+ public void checkUnionGo() { deserializeUnionResults(GroupLanguage.GO); }
+
+ @Test(groups = {CHECK_RUST_FILES})
+ public void checkUnionRust() { deserializeUnionResults(GroupLanguage.RUST); }
+
+ /**
+ * A union whose inputs span all three curModes, so the coupon-update path
into a gadget with
+ * a pending KxQ rebuild is exercised. The second sketch stays in SET mode
and overlaps the
+ * first entirely, so the true distinct count is UNION_DISTINCT.
+ */
+ private static HllUnion mixedModeUnion() {
+ final HllUnion union = new HllUnion(LG_K);
+ final HllSketch big = new HllSketch(LG_K, HLL_8);
+ for (int i = 0; i < 1000; i++) { big.update(i); }
+ final HllSketch small = new HllSketch(LG_K, HLL_8);
+ for (int i = 0; i < 20; i++) { small.update(500 + i); } //stays in SET
mode, fully overlapping
+ final HllSketch other = new HllSketch(LG_K, HLL_8);
+ for (int i = 0; i < 5000; i++) { other.update(2000 + i); }
+ union.update(big);
+ union.update(small);
+ union.update(other);
+ return union;
+ }
+
+ private static String tagOf(final TgtHllType type) {
+ return type == HLL_4 ? "hll4" : type == HLL_6 ? "hll6" : "hll8";
+ }
+
+ /**
+ * One body, reused by every language. Beyond deserializing, this asserts
the two flags-byte
+ * invariants the 2026-09 fixes established: bit 32 is reserved and must
never be written by
+ * any implementation, and the compact flag must reflect the form actually
produced.
+ */
+ private static void deserializeUnionResults(final GroupLanguage lang) {
+ final String[] tags = {"hll4", "hll6", "hll8"};
+ final String[] forms = {"_union", "_unionupd"};
+ double firstEstimate = Double.NaN;
+ for (final String form: forms) {
+ for (final String tag: tags) {
+ final String fileName = tag + form + lang.sfx + ".sk";
+ final byte[] bytes = getFileBytes(lang.pth, fileName);
+ if (bytes.length == 0) { continue; }
+ final int flags = bytes[FLAGS_BYTE] & 0xFF;
+
+ //bit 32 collided across implementations: REBUILD_CURMIN_NUM_KXQ here,
FULL_SIZE in C++.
+ //Java clears it before serializing and C++ no longer writes it, so no
image carries it.
+ assertFalse((flags & REBUILD_CURMIN_NUM_KXQ_MASK) > 0,
+ fileName + " has reserved flags bit 32 set");
+
+ //HLL_6 and HLL_8 used to omit this on toCompactByteArray()
+ assertEquals((flags & COMPACT_FLAG_MASK) > 0, form.equals("_union"),
+ fileName + " compact flag does not match its serialized form");
+
+ final HllSketch sketch =
HllSketch.heapify(MemorySegment.ofArray(bytes));
+ assertEquals(sketch.getLgConfigK(), LG_K);
+ assertTrue(!sketch.isEmpty());
+ assertEquals(sketch.getEstimate(), UNION_DISTINCT, UNION_DISTINCT *
0.02);
+
Review Comment:
For more robust compatibility test, how about adding comparing "curMin",
"numAtCurMin"?
--
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]
