Skip to content

Commit ba29502

Browse files
authored
Merge pull request #529 from apache/5.0.X-patch
prep for 5.0.2, fix KllItemsSketch level 0 sorting issue
2 parents 4a020cc + 806bd4b commit ba29502

File tree

3 files changed

+34
-4
lines changed

3 files changed

+34
-4
lines changed

pom.xml

+2-2
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ under the License.
3333

3434
<groupId>org.apache.datasketches</groupId>
3535
<artifactId>datasketches-java</artifactId>
36-
<version>5.0.1</version>
36+
<version>5.0.2</version>
3737
<packaging>jar</packaging>
3838

3939
<name>${project.artifactId}</name>
@@ -734,7 +734,7 @@ under the License.
734734
</pluginManagement>
735735
</build>
736736
</profile>
737-
737+
738738
<profile>
739739
<id>check-cpp-historical-files</id>
740740
<build>

src/main/java/org/apache/datasketches/kll/KllItemsHelper.java

+1-2
Original file line numberDiff line numberDiff line change
@@ -409,7 +409,7 @@ private static <T> int[] generalItemsCompress(
409409

410410
// level zero might not be sorted, so we must sort it if we wish to compact it
411411
if ((curLevel == 0) && !isLevelZeroSorted) {
412-
Arrays.sort(inBuf, adjBeg, adjBeg + adjPop);
412+
Arrays.sort((T[])inBuf, adjBeg, adjBeg + adjPop, comp);
413413
}
414414

415415
if (popAbove == 0) { // Level above is empty, so halve up
@@ -486,4 +486,3 @@ private static <T> void populateItemWorkArrays(
486486
// }
487487

488488
}
489-

src/test/java/org/apache/datasketches/kll/KllItemsSketchTest.java

+31
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import static java.lang.Math.ceil;
2323
import static org.apache.datasketches.kll.KllSketch.SketchStructure.*;
2424
import static org.apache.datasketches.kll.KllSketch.SketchType.*;
25+
import static org.apache.datasketches.quantilescommon.LongsAsOrderableStrings.getString;
2526
import static org.apache.datasketches.quantilescommon.QuantileSearchCriteria.EXCLUSIVE;
2627
import static org.apache.datasketches.quantilescommon.QuantileSearchCriteria.INCLUSIVE;
2728
import static org.testng.Assert.assertEquals;
@@ -31,6 +32,7 @@
3132
import static org.testng.Assert.fail;
3233

3334
import java.util.Comparator;
35+
import java.util.Random;
3436

3537
import org.apache.datasketches.common.ArrayOfStringsSerDe;
3638
import org.apache.datasketches.common.SketchesArgumentException;
@@ -42,6 +44,7 @@
4244
import org.apache.datasketches.quantilescommon.DoublesSortedView;
4345
import org.apache.datasketches.quantilescommon.GenericSortedView;
4446
import org.apache.datasketches.quantilescommon.GenericSortedViewIterator;
47+
import org.apache.datasketches.quantilescommon.QuantilesGenericSketchIterator;
4548
import org.testng.annotations.Test;
4649

4750
@SuppressWarnings("unused")
@@ -751,6 +754,34 @@ public void checkSortedViewAfterReset() {
751754
try { sk.getSortedView(); fail(); } catch (SketchesArgumentException e) { }
752755
}
753756

757+
@Test
758+
//There is no guarantee that L0 is sorted after a merge.
759+
//The issue is, during a merge, L0 must be sorted prior to a compaction to a higher level.
760+
//Otherwise the higher levels would not be sorted properly.
761+
public void checkL0SortDuringMerge() throws NumberFormatException {
762+
final Random rand = new Random();
763+
final KllItemsSketch<String> sk1 = KllItemsSketch.newHeapInstance(8, Comparator.reverseOrder(), serDe);
764+
final KllItemsSketch<String> sk2 = KllItemsSketch.newHeapInstance(8, Comparator.reverseOrder(), serDe);
765+
final int n = 26; //don't change this
766+
for (int i = 1; i <= n; i++ ) {
767+
final int j = rand.nextInt(n) + 1;
768+
sk1.update(getString(j, 3));
769+
sk2.update(getString(j +100, 3));
770+
}
771+
sk1.merge(sk2);
772+
println(sk1.toString(true, true)); //L1 and above should be sorted in reverse. Ignore L0.
773+
final int lvl1size = sk1.levelsArr[2] - sk1.levelsArr[1];
774+
final QuantilesGenericSketchIterator<String> itr = sk1.iterator();
775+
itr.next();
776+
int prev = Integer.parseInt(itr.getQuantile().trim());
777+
for (int i = 1; i < lvl1size; i++) {
778+
if (itr.next()) {
779+
int v = Integer.parseInt(itr.getQuantile().trim());
780+
assertTrue(v <= prev);
781+
prev = v;
782+
}
783+
}
784+
}
754785

755786
private final static boolean enablePrinting = false;
756787

0 commit comments

Comments
 (0)