diff --git a/src/main/java/org/apache/datasketches/kll/KllItemsSketch.java b/src/main/java/org/apache/datasketches/kll/KllItemsSketch.java index 89f277fb8..d71c75a85 100644 --- a/src/main/java/org/apache/datasketches/kll/KllItemsSketch.java +++ b/src/main/java/org/apache/datasketches/kll/KllItemsSketch.java @@ -426,7 +426,10 @@ ItemsSketchSortedView getSV() { if (!isLevelZeroSorted()) { Arrays.sort(srcQuantiles, srcLevelsArr[0], srcLevelsArr[1], comparator); - if (!hasMemorySegment()) { setLevelZeroSorted(true); } + // Do not setLevelZeroSorted(true). Unlike KllDoublesSketch, getTotalItemsArray() + // returns a defensive copy for the heap Items sketch (and a reconstituted array for + // direct), so this sort does not sort the live level-0. Setting the flag would make + // serialization claim level-0 is sorted and cause heapify/wrap to skip sorting (#756). } final int numQuantiles = getNumRetained(); quantiles = (T[]) Array.newInstance(serDe.getClassOfT(), numQuantiles); diff --git a/src/test/java/org/apache/datasketches/kll/KllItemsSketchSerDeTest.java b/src/test/java/org/apache/datasketches/kll/KllItemsSketchSerDeTest.java index 0e5384286..e8a04ea30 100644 --- a/src/test/java/org/apache/datasketches/kll/KllItemsSketchSerDeTest.java +++ b/src/test/java/org/apache/datasketches/kll/KllItemsSketchSerDeTest.java @@ -19,6 +19,7 @@ package org.apache.datasketches.kll; +import static org.apache.datasketches.quantilescommon.QuantileSearchCriteria.INCLUSIVE; import static org.testng.Assert.assertEquals; import static org.testng.Assert.assertFalse; import static org.testng.Assert.assertTrue; @@ -126,4 +127,68 @@ public void serializeDeserializeMultipleValues() { assertEquals(bytes, bytes2); } + @Test + // Querying a heap Items sketch before serialization must not corrupt the round-trip + // sorted view. CreateSortedView sorted a defensive copy from getTotalItemsArray() but + // still set levelZeroSorted, so heapify/wrap skipped sorting (#756). + public void serializeDeserializeAfterQueryHeapifyMatches() { + final KllItemsSketch sk = KllItemsSketch.newHeapInstance(8, Comparator.naturalOrder(), serDe); + sk.update("a"); + sk.update("b"); + sk.update("c"); + sk.update("d"); + assertFalse(sk.isLevelZeroSorted()); + sk.getQuantile(0.5, INCLUSIVE); // any query builds the sorted view + assertFalse(sk.isLevelZeroSorted()); // live level-0 was not sorted + + final KllItemsSketch rt = KllItemsSketch.heapify( + MemorySegment.ofArray(sk.toByteArray()), Comparator.naturalOrder(), serDe); + assertEquals(rt.getNumRetained(), sk.getNumRetained()); + assertEquals(rt.getSortedView().getQuantiles(), sk.getSortedView().getQuantiles()); + assertEquals(rt.getSortedView().getCumulativeWeights(), sk.getSortedView().getCumulativeWeights()); + for (int i = 0; i <= 20; i++) { + final double rank = i / 20.0; + assertEquals(rt.getQuantile(rank, INCLUSIVE), sk.getQuantile(rank, INCLUSIVE), + "rank=" + rank); + } + } + + @Test + public void serializeDeserializeAfterQueryWrapMatches() { + final KllItemsSketch sk = KllItemsSketch.newHeapInstance(8, Comparator.naturalOrder(), serDe); + sk.update("a"); + sk.update("b"); + sk.update("c"); + sk.update("d"); + sk.getQuantile(0.5, INCLUSIVE); + + final KllItemsSketch rt = KllItemsSketch.wrap( + MemorySegment.ofArray(sk.toByteArray()), Comparator.naturalOrder(), serDe); + assertEquals(rt.getSortedView().getQuantiles(), sk.getSortedView().getQuantiles()); + assertEquals(rt.getSortedView().getCumulativeWeights(), sk.getSortedView().getCumulativeWeights()); + assertEquals(rt.getQuantile(0.5, INCLUSIVE), sk.getQuantile(0.5, INCLUSIVE)); + assertEquals(rt.getQuantile(0.55, INCLUSIVE), sk.getQuantile(0.55, INCLUSIVE)); + } + + @Test + public void serializeDeserializeAfterQueryWithCompaction() { + final KllItemsSketch sk = KllItemsSketch.newHeapInstance(8, Comparator.naturalOrder(), serDe); + for (int i = 1; i <= 8; i++) { + sk.update(String.valueOf((char) ('a' + i - 1))); + } + sk.getQuantile(0.5, INCLUSIVE); + assertFalse(sk.isLevelZeroSorted()); + + final KllItemsSketch heapified = KllItemsSketch.heapify( + MemorySegment.ofArray(sk.toByteArray()), Comparator.naturalOrder(), serDe); + final KllItemsSketch wrapped = KllItemsSketch.wrap( + MemorySegment.ofArray(sk.toByteArray()), Comparator.naturalOrder(), serDe); + for (int i = 0; i <= 20; i++) { + final double rank = i / 20.0; + final String expected = sk.getQuantile(rank, INCLUSIVE); + assertEquals(heapified.getQuantile(rank, INCLUSIVE), expected, "heapify rank=" + rank); + assertEquals(wrapped.getQuantile(rank, INCLUSIVE), expected, "wrap rank=" + rank); + } + } + }