From f6cb6eb34fd54109b70aad593b10bbfbd304509b Mon Sep 17 00:00:00 2001 From: Lee Rhodes Date: Sat, 26 Sep 2026 15:13:06 -0700 Subject: [PATCH 1/2] AOD compact sketch: zero seed hash for empty images, ignore it on read Follow what Java theta does: an empty compact ArrayOfDoubles image carries a zero seed hash, and readers ignore those bytes on an empty image, so historical images that filled them in still load. Set operations skip the seed-hash check for empty inputs, as the theta set operations do. C++ already writes a zero seed hash for an empty compact tuple sketch (datasketches-cpp c11a5ea), which Java rejected in AodSketchCrossLanguageTest.checkCpp. Co-Authored-By: Claude Opus 5.5 --- .../ArrayOfDoublesAnotBImpl.java | 11 +++-- .../ArrayOfDoublesIntersection.java | 2 +- .../arrayofdoubles/ArrayOfDoublesUnion.java | 2 +- .../DirectArrayOfDoublesCompactSketch.java | 10 ++-- .../HeapArrayOfDoublesCompactSketch.java | 6 ++- .../ArrayOfDoublesAnotBTest.java | 6 ++- .../ArrayOfDoublesIntersectionTest.java | 1 + .../ArrayOfDoublesUnionTest.java | 1 + .../HeapArrayOfDoublesCompactSketchTest.java | 48 +++++++++++++++++++ 9 files changed, 74 insertions(+), 13 deletions(-) diff --git a/src/main/java/org/apache/datasketches/tuple/arrayofdoubles/ArrayOfDoublesAnotBImpl.java b/src/main/java/org/apache/datasketches/tuple/arrayofdoubles/ArrayOfDoublesAnotBImpl.java index bec533a04..3067fc3f4 100644 --- a/src/main/java/org/apache/datasketches/tuple/arrayofdoubles/ArrayOfDoublesAnotBImpl.java +++ b/src/main/java/org/apache/datasketches/tuple/arrayofdoubles/ArrayOfDoublesAnotBImpl.java @@ -70,13 +70,9 @@ public void update(final ArrayOfDoublesSketch skA, final ArrayOfDoublesSketch sk throw new SketchesArgumentException("Neither argument may be null."); } numValues_ = skA.getNumValues(); - seedHash_ = skA.getSeedHash(); if (numValues_ != skB.getNumValues()) { throw new SketchesArgumentException("Inputs cannot have different numValues"); } - if (seedHash_ != skB.getSeedHash()) { - throw new SketchesArgumentException("Inputs cannot have different seedHashes"); - } final long thetaLongA = skA.getThetaLong(); final int countA = skA.getRetainedEntries(); @@ -86,6 +82,13 @@ public void update(final ArrayOfDoublesSketch skA, final ArrayOfDoublesSketch sk final int countB = skB.getRetainedEntries(); final boolean emptyB = skB.isEmpty(); + // the seed hash of an empty sketch is not meaningful (its image carries zero) + if (!emptyA) { seedHash_ = skA.getSeedHash(); } + else if (!emptyB) { seedHash_ = skB.getSeedHash(); } + if (!emptyA && !emptyB && (skA.getSeedHash() != skB.getSeedHash())) { + throw new SketchesArgumentException("Inputs cannot have different seedHashes"); + } + final int id = SetOperationCornerCases.createCornerCaseId(thetaLongA, countA, emptyA, thetaLongB, countB, emptyB); final CornerCase cCase = CornerCase.caseIdToCornerCase(id); diff --git a/src/main/java/org/apache/datasketches/tuple/arrayofdoubles/ArrayOfDoublesIntersection.java b/src/main/java/org/apache/datasketches/tuple/arrayofdoubles/ArrayOfDoublesIntersection.java index 91d51b483..74db11652 100644 --- a/src/main/java/org/apache/datasketches/tuple/arrayofdoubles/ArrayOfDoublesIntersection.java +++ b/src/main/java/org/apache/datasketches/tuple/arrayofdoubles/ArrayOfDoublesIntersection.java @@ -65,7 +65,7 @@ public abstract class ArrayOfDoublesIntersection { */ public void intersect(final ArrayOfDoublesSketch tupleSketch, final ArrayOfDoublesCombiner combiner) { if (tupleSketch == null) { throw new SketchesArgumentException("TupleSketch must not be null"); } - Util.checkSeedHashes(seedHash_, tupleSketch.getSeedHash()); + if (!tupleSketch.isEmpty()) { Util.checkSeedHashes(seedHash_, tupleSketch.getSeedHash()); } if (tupleSketch.numValues_ != numValues_) { throw new SketchesArgumentException( "Input TupleSketch cannot have different numValues from the internal numValues."); diff --git a/src/main/java/org/apache/datasketches/tuple/arrayofdoubles/ArrayOfDoublesUnion.java b/src/main/java/org/apache/datasketches/tuple/arrayofdoubles/ArrayOfDoublesUnion.java index 2d11551ee..221abf976 100644 --- a/src/main/java/org/apache/datasketches/tuple/arrayofdoubles/ArrayOfDoublesUnion.java +++ b/src/main/java/org/apache/datasketches/tuple/arrayofdoubles/ArrayOfDoublesUnion.java @@ -109,7 +109,7 @@ public static ArrayOfDoublesUnion wrap(final MemorySegment srcSeg, final long se */ public void union(final ArrayOfDoublesSketch tupleSketch) { if (tupleSketch == null) { return; } - Util.checkSeedHashes(gadget_.getSeedHash(), tupleSketch.getSeedHash()); + if (!tupleSketch.isEmpty()) { Util.checkSeedHashes(gadget_.getSeedHash(), tupleSketch.getSeedHash()); } if (gadget_.getNumValues() != tupleSketch.getNumValues()) { throw new SketchesArgumentException("Incompatible sketches: number of values mismatch " + gadget_.getNumValues() + " and " + tupleSketch.getNumValues()); diff --git a/src/main/java/org/apache/datasketches/tuple/arrayofdoubles/DirectArrayOfDoublesCompactSketch.java b/src/main/java/org/apache/datasketches/tuple/arrayofdoubles/DirectArrayOfDoublesCompactSketch.java index 76883224e..318fa59fb 100644 --- a/src/main/java/org/apache/datasketches/tuple/arrayofdoubles/DirectArrayOfDoublesCompactSketch.java +++ b/src/main/java/org/apache/datasketches/tuple/arrayofdoubles/DirectArrayOfDoublesCompactSketch.java @@ -78,7 +78,8 @@ final class DirectArrayOfDoublesCompactSketch extends ArrayOfDoublesCompactSketc | (count > 0 ? 1 << Flags.HAS_ENTRIES.ordinal() : 0) )); dstSeg.set(JAVA_BYTE, NUM_VALUES_BYTE, (byte) numValues_); - dstSeg.set(JAVA_SHORT_UNALIGNED, SEED_HASH_SHORT, Util.computeSeedHash(sketch.getSeed())); + // an empty compact image carries a zero seed hash, which readers ignore + dstSeg.set(JAVA_SHORT_UNALIGNED, SEED_HASH_SHORT, isEmpty_ ? 0 : Util.computeSeedHash(sketch.getSeed())); thetaLong_ = Math.min(sketch.getThetaLong(), thetaLong); dstSeg.set(JAVA_LONG_UNALIGNED, THETA_LONG, thetaLong_); if (count > 0) { @@ -119,7 +120,7 @@ final class DirectArrayOfDoublesCompactSketch extends ArrayOfDoublesCompactSketc | (count > 0 ? 1 << Flags.HAS_ENTRIES.ordinal() : 0) )); dstSeg.set(JAVA_BYTE, NUM_VALUES_BYTE, (byte) numValues_); - dstSeg.set(JAVA_SHORT_UNALIGNED, SEED_HASH_SHORT, seedHash); + dstSeg.set(JAVA_SHORT_UNALIGNED, SEED_HASH_SHORT, isEmpty_ ? 0 : seedHash); thetaLong_ = thetaLong; dstSeg.set(JAVA_LONG_UNALIGNED, THETA_LONG, thetaLong_); if (count > 0) { @@ -167,8 +168,11 @@ final class DirectArrayOfDoublesCompactSketch extends ArrayOfDoublesCompactSketc throw new SketchesArgumentException("Serial version mismatch. Expected: " + serialVersionUID + ", actual: " + version); } - Util.checkSeedHashes(seg.get(JAVA_SHORT_UNALIGNED, SEED_HASH_SHORT), Util.computeSeedHash(seed)); isEmpty_ = (seg_.get(JAVA_BYTE, FLAGS_BYTE) & (1 << Flags.IS_EMPTY.ordinal())) != 0; + // the seed hash of an empty image is ignored; historical images may have filled it in + if (!isEmpty_) { + Util.checkSeedHashes(seg.get(JAVA_SHORT_UNALIGNED, SEED_HASH_SHORT), Util.computeSeedHash(seed)); + } thetaLong_ = seg.get(JAVA_LONG_UNALIGNED, THETA_LONG); } diff --git a/src/main/java/org/apache/datasketches/tuple/arrayofdoubles/HeapArrayOfDoublesCompactSketch.java b/src/main/java/org/apache/datasketches/tuple/arrayofdoubles/HeapArrayOfDoublesCompactSketch.java index 8672014d4..862631e2a 100644 --- a/src/main/java/org/apache/datasketches/tuple/arrayofdoubles/HeapArrayOfDoublesCompactSketch.java +++ b/src/main/java/org/apache/datasketches/tuple/arrayofdoubles/HeapArrayOfDoublesCompactSketch.java @@ -126,8 +126,9 @@ final class HeapArrayOfDoublesCompactSketch extends ArrayOfDoublesCompactSketch throw new SketchesArgumentException( "Serial version mismatch. Expected: " + serialVersionUID + ", actual: " + version); } - Util.checkSeedHashes(seedHash_, Util.computeSeedHash(seed)); isEmpty_ = (seg.get(JAVA_BYTE, FLAGS_BYTE) & (1 << Flags.IS_EMPTY.ordinal())) != 0; + // the seed hash of an empty image is ignored; historical images may have filled it in + if (!isEmpty_) { Util.checkSeedHashes(seedHash_, Util.computeSeedHash(seed)); } thetaLong_ = seg.get(JAVA_LONG_UNALIGNED, THETA_LONG); final boolean hasEntries = (seg.get(JAVA_BYTE, FLAGS_BYTE) & (1 << Flags.HAS_ENTRIES.ordinal())) != 0; @@ -172,7 +173,8 @@ public byte[] toByteArray() { | ((count > 0 ? 1 : 0) << Flags.HAS_ENTRIES.ordinal()) )); seg.set(JAVA_BYTE, NUM_VALUES_BYTE, (byte) numValues_); - seg.set(JAVA_SHORT_UNALIGNED, SEED_HASH_SHORT, seedHash_); + // an empty compact image carries a zero seed hash, which readers ignore + seg.set(JAVA_SHORT_UNALIGNED, SEED_HASH_SHORT, isEmpty() ? 0 : seedHash_); seg.set(JAVA_LONG_UNALIGNED, THETA_LONG, thetaLong_); if (count > 0) { seg.set(JAVA_INT_UNALIGNED, RETAINED_ENTRIES_INT, count); diff --git a/src/test/java/org/apache/datasketches/tuple/arrayofdoubles/ArrayOfDoublesAnotBTest.java b/src/test/java/org/apache/datasketches/tuple/arrayofdoubles/ArrayOfDoublesAnotBTest.java index 60dad7dec..734c413f7 100644 --- a/src/test/java/org/apache/datasketches/tuple/arrayofdoubles/ArrayOfDoublesAnotBTest.java +++ b/src/test/java/org/apache/datasketches/tuple/arrayofdoubles/ArrayOfDoublesAnotBTest.java @@ -306,8 +306,10 @@ public void incompatibleSeedB() { @Test(expectedExceptions = SketchesArgumentException.class) public void incompatibleSeeds() { - ArrayOfDoublesSketch sketchA = new ArrayOfDoublesUpdatableSketchBuilder().setSeed(1).build(); - ArrayOfDoublesSketch sketchB = new ArrayOfDoublesUpdatableSketchBuilder().setSeed(2).build(); + ArrayOfDoublesUpdatableSketch sketchA = new ArrayOfDoublesUpdatableSketchBuilder().setSeed(1).build(); + ArrayOfDoublesUpdatableSketch sketchB = new ArrayOfDoublesUpdatableSketchBuilder().setSeed(2).build(); + sketchA.update(1, new double[] {1.0}); // the seed hash of an empty sketch is not checked + sketchB.update(1, new double[] {1.0}); ArrayOfDoublesAnotB aNotB = new ArrayOfDoublesSetOperationBuilder().setSeed(3).buildAnotB(); aNotB.update(sketchA, sketchB); } diff --git a/src/test/java/org/apache/datasketches/tuple/arrayofdoubles/ArrayOfDoublesIntersectionTest.java b/src/test/java/org/apache/datasketches/tuple/arrayofdoubles/ArrayOfDoublesIntersectionTest.java index f30c92bb5..3871bbc87 100644 --- a/src/test/java/org/apache/datasketches/tuple/arrayofdoubles/ArrayOfDoublesIntersectionTest.java +++ b/src/test/java/org/apache/datasketches/tuple/arrayofdoubles/ArrayOfDoublesIntersectionTest.java @@ -305,6 +305,7 @@ public void heapExactModeCustomSeed() { @Test(expectedExceptions = SketchesArgumentException.class) public void incompatibleSeeds() { final ArrayOfDoublesUpdatableSketch sketch = new ArrayOfDoublesUpdatableSketchBuilder().setSeed(1).build(); + sketch.update(1, new double[] {1.0}); // the seed hash of an empty sketch is not checked final ArrayOfDoublesIntersection intersection = new ArrayOfDoublesSetOperationBuilder().setSeed(2).buildIntersection(); intersection.intersect(sketch, combiner); } diff --git a/src/test/java/org/apache/datasketches/tuple/arrayofdoubles/ArrayOfDoublesUnionTest.java b/src/test/java/org/apache/datasketches/tuple/arrayofdoubles/ArrayOfDoublesUnionTest.java index d9b02f768..ef961aa81 100644 --- a/src/test/java/org/apache/datasketches/tuple/arrayofdoubles/ArrayOfDoublesUnionTest.java +++ b/src/test/java/org/apache/datasketches/tuple/arrayofdoubles/ArrayOfDoublesUnionTest.java @@ -426,6 +426,7 @@ public void directToHeap() { @Test(expectedExceptions = SketchesArgumentException.class) public void incompatibleSeeds() { final ArrayOfDoublesUpdatableSketch sketch = new ArrayOfDoublesUpdatableSketchBuilder().setSeed(1).build(); + sketch.update(1, new double[] {1.0}); // the seed hash of an empty sketch is not checked final ArrayOfDoublesUnion union = new ArrayOfDoublesSetOperationBuilder().setSeed(2).buildUnion(); union.union(sketch); } diff --git a/src/test/java/org/apache/datasketches/tuple/arrayofdoubles/HeapArrayOfDoublesCompactSketchTest.java b/src/test/java/org/apache/datasketches/tuple/arrayofdoubles/HeapArrayOfDoublesCompactSketchTest.java index 7c8c038cf..69e718220 100644 --- a/src/test/java/org/apache/datasketches/tuple/arrayofdoubles/HeapArrayOfDoublesCompactSketchTest.java +++ b/src/test/java/org/apache/datasketches/tuple/arrayofdoubles/HeapArrayOfDoublesCompactSketchTest.java @@ -134,4 +134,52 @@ public void deserializeWithWrongSeed() { MemorySegment seg = MemorySegment.ofArray(sketch1.toByteArray()); ArrayOfDoublesSketch.heapifySketch(seg, 123); } + @Test + public void emptyImageSeedHashIsZeroAndIgnoredOnRead() { + final long seed = 12345; + final ArrayOfDoublesUpdatableSketch us = new ArrayOfDoublesUpdatableSketchBuilder().setSeed(seed).build(); + final byte[] heapBytes = us.compact().toByteArray(); + final byte[] directBytes = new byte[heapBytes.length + 8]; // the direct writer asks for room for one entry + us.compact(MemorySegment.ofArray(directBytes)); + Assert.assertEquals(java.util.Arrays.copyOf(directBytes, heapBytes.length), heapBytes); + Assert.assertEquals(MemorySegment.ofArray(heapBytes).get(java.lang.foreign.ValueLayout.JAVA_SHORT_UNALIGNED, + ArrayOfDoublesSketch.SEED_HASH_SHORT), (short) 0); + + // a historical empty image may carry a seed hash; it is ignored on read + final byte[] historical = heapBytes.clone(); + MemorySegment.ofArray(historical).set(java.lang.foreign.ValueLayout.JAVA_SHORT_UNALIGNED, + ArrayOfDoublesSketch.SEED_HASH_SHORT, (short) 0x1234); + for (final byte[] bytes : new byte[][] {heapBytes, historical}) { + final ArrayOfDoublesSketch heap = new HeapArrayOfDoublesCompactSketch(MemorySegment.ofArray(bytes), seed); + final ArrayOfDoublesSketch direct = new DirectArrayOfDoublesCompactSketch(MemorySegment.ofArray(bytes), seed); + Assert.assertEquals(heap.toByteArray(), heapBytes); // re-serializing writes a zero seed hash + for (final ArrayOfDoublesSketch empty : new ArrayOfDoublesSketch[] {heap, direct}) { + Assert.assertTrue(empty.isEmpty()); + + // set operations ignore the seed hash of an empty input + final ArrayOfDoublesUpdatableSketch other = new ArrayOfDoublesUpdatableSketchBuilder().setSeed(seed).build(); + other.update(1, new double[] {1.0}); + final ArrayOfDoublesUnion union = new ArrayOfDoublesSetOperationBuilder().setSeed(seed).buildUnion(); + union.union(empty); + union.union(other); + Assert.assertEquals(union.getResult().getEstimate(), 1.0); + final ArrayOfDoublesIntersection inter = new ArrayOfDoublesSetOperationBuilder().setSeed(seed).buildIntersection(); + inter.intersect(empty, null); + Assert.assertTrue(inter.getResult().isEmpty()); + final ArrayOfDoublesAnotB aNotB = new ArrayOfDoublesSetOperationBuilder().setSeed(seed).buildAnotB(); + aNotB.update(other, empty); + Assert.assertEquals(aNotB.getResult().getEstimate(), 1.0); + aNotB.update(empty, other); + Assert.assertTrue(aNotB.getResult().isEmpty()); + } + } + + // a non-empty image still has its seed hash checked + us.update(1, new double[] {1.0}); + final byte[] withEntry = us.compact().toByteArray(); + Assert.assertThrows(SketchesArgumentException.class, + () -> new HeapArrayOfDoublesCompactSketch(MemorySegment.ofArray(withEntry), seed + 1)); + Assert.assertThrows(SketchesArgumentException.class, + () -> new DirectArrayOfDoublesCompactSketch(MemorySegment.ofArray(withEntry), seed + 1)); + } } From f63e080bd1f88d7511e6ed55e33e905db9de1f5e Mon Sep 17 00:00:00 2001 From: Lee Rhodes Date: Mon, 2 Nov 2026 09:54:00 -0800 Subject: [PATCH 2/2] AoD compact sketch: check the seed hash of an empty image that has entries An image is skipped by the seed hash check only when it is empty and has no entries. A corrupt image that sets both the empty and has-entries flags is still checked, so its entries can't bypass the check. Applies to the heap and direct readers. As suggested in review. Co-Authored-By: Claude Opus 5.5 --- .../arrayofdoubles/DirectArrayOfDoublesCompactSketch.java | 6 ++++-- .../arrayofdoubles/HeapArrayOfDoublesCompactSketch.java | 7 ++++--- .../HeapArrayOfDoublesCompactSketchTest.java | 8 ++++++++ 3 files changed, 16 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/apache/datasketches/tuple/arrayofdoubles/DirectArrayOfDoublesCompactSketch.java b/src/main/java/org/apache/datasketches/tuple/arrayofdoubles/DirectArrayOfDoublesCompactSketch.java index 318fa59fb..e110a24fa 100644 --- a/src/main/java/org/apache/datasketches/tuple/arrayofdoubles/DirectArrayOfDoublesCompactSketch.java +++ b/src/main/java/org/apache/datasketches/tuple/arrayofdoubles/DirectArrayOfDoublesCompactSketch.java @@ -169,8 +169,10 @@ final class DirectArrayOfDoublesCompactSketch extends ArrayOfDoublesCompactSketc + ", actual: " + version); } isEmpty_ = (seg_.get(JAVA_BYTE, FLAGS_BYTE) & (1 << Flags.IS_EMPTY.ordinal())) != 0; - // the seed hash of an empty image is ignored; historical images may have filled it in - if (!isEmpty_) { + final boolean hasEntries = (seg_.get(JAVA_BYTE, FLAGS_BYTE) & (1 << Flags.HAS_ENTRIES.ordinal())) != 0; + // the seed hash of an empty image is ignored; historical images may have filled it in. + // A corrupt image that claims to be empty but has entries is still checked. + if (!isEmpty_ || hasEntries) { Util.checkSeedHashes(seg.get(JAVA_SHORT_UNALIGNED, SEED_HASH_SHORT), Util.computeSeedHash(seed)); } thetaLong_ = seg.get(JAVA_LONG_UNALIGNED, THETA_LONG); diff --git a/src/main/java/org/apache/datasketches/tuple/arrayofdoubles/HeapArrayOfDoublesCompactSketch.java b/src/main/java/org/apache/datasketches/tuple/arrayofdoubles/HeapArrayOfDoublesCompactSketch.java index 862631e2a..762324543 100644 --- a/src/main/java/org/apache/datasketches/tuple/arrayofdoubles/HeapArrayOfDoublesCompactSketch.java +++ b/src/main/java/org/apache/datasketches/tuple/arrayofdoubles/HeapArrayOfDoublesCompactSketch.java @@ -127,11 +127,12 @@ final class HeapArrayOfDoublesCompactSketch extends ArrayOfDoublesCompactSketch "Serial version mismatch. Expected: " + serialVersionUID + ", actual: " + version); } isEmpty_ = (seg.get(JAVA_BYTE, FLAGS_BYTE) & (1 << Flags.IS_EMPTY.ordinal())) != 0; - // the seed hash of an empty image is ignored; historical images may have filled it in - if (!isEmpty_) { Util.checkSeedHashes(seedHash_, Util.computeSeedHash(seed)); } - thetaLong_ = seg.get(JAVA_LONG_UNALIGNED, THETA_LONG); final boolean hasEntries = (seg.get(JAVA_BYTE, FLAGS_BYTE) & (1 << Flags.HAS_ENTRIES.ordinal())) != 0; + // the seed hash of an empty image is ignored; historical images may have filled it in. + // A corrupt image that claims to be empty but has entries is still checked. + if (!isEmpty_ || hasEntries) { Util.checkSeedHashes(seedHash_, Util.computeSeedHash(seed)); } + thetaLong_ = seg.get(JAVA_LONG_UNALIGNED, THETA_LONG); if (hasEntries) { final int count = seg.get(JAVA_INT_UNALIGNED, RETAINED_ENTRIES_INT); keys_ = new long[count]; diff --git a/src/test/java/org/apache/datasketches/tuple/arrayofdoubles/HeapArrayOfDoublesCompactSketchTest.java b/src/test/java/org/apache/datasketches/tuple/arrayofdoubles/HeapArrayOfDoublesCompactSketchTest.java index 69e718220..f01577b86 100644 --- a/src/test/java/org/apache/datasketches/tuple/arrayofdoubles/HeapArrayOfDoublesCompactSketchTest.java +++ b/src/test/java/org/apache/datasketches/tuple/arrayofdoubles/HeapArrayOfDoublesCompactSketchTest.java @@ -181,5 +181,13 @@ public void emptyImageSeedHashIsZeroAndIgnoredOnRead() { () -> new HeapArrayOfDoublesCompactSketch(MemorySegment.ofArray(withEntry), seed + 1)); Assert.assertThrows(SketchesArgumentException.class, () -> new DirectArrayOfDoublesCompactSketch(MemorySegment.ofArray(withEntry), seed + 1)); + + // so is a corrupt image that claims to be empty but has entries + final byte[] emptyWithEntry = withEntry.clone(); + emptyWithEntry[ArrayOfDoublesSketch.FLAGS_BYTE] |= (byte) (1 << ArrayOfDoublesSketch.Flags.IS_EMPTY.ordinal()); + Assert.assertThrows(SketchesArgumentException.class, + () -> new HeapArrayOfDoublesCompactSketch(MemorySegment.ofArray(emptyWithEntry), seed + 1)); + Assert.assertThrows(SketchesArgumentException.class, + () -> new DirectArrayOfDoublesCompactSketch(MemorySegment.ofArray(emptyWithEntry), seed + 1)); } }