Skip to content

Commit e2b9d3f

Browse files
committed
Fix Issue 484.
The serDe of min and max were mingled with the serDe of Boolean Items and they should not have been. The fix was in two places. Added a test for this issue.
1 parent 4d43fc3 commit e2b9d3f

5 files changed

Lines changed: 39 additions & 4 deletions

File tree

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import java.lang.reflect.Array;
3333
import java.util.Comparator;
3434

35+
import org.apache.datasketches.common.ArrayOfBooleansSerDe;
3536
import org.apache.datasketches.common.ArrayOfItemsSerDe;
3637
import org.apache.datasketches.common.SketchesArgumentException;
3738
import org.apache.datasketches.memory.Memory;
@@ -154,6 +155,7 @@ byte[] getMinMaxByteArr() { //this is only used by COMPACT_FULL
154155
@Override
155156
int getMinMaxSizeBytes() { //this is only used by COMPACT_FULL
156157
final int offset = DATA_START_ADR + getNumLevels() * Integer.BYTES;
158+
if (serDe instanceof ArrayOfBooleansSerDe) { return 2; }
157159
return serDe.sizeOf(mem, offset, 2);
158160
}
159161

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -432,13 +432,13 @@ static byte[] toByteArray(final KllSketch srcSk, final boolean updatable) {
432432
final byte m = (byte) srcSk.getM();
433433

434434
//load first 8 bytes
435-
wbuf.putByte(preInts);
435+
wbuf.putByte(preInts); //byte 0
436436
wbuf.putByte(serVer);
437437
wbuf.putByte(famId);
438438
wbuf.putByte(flags);
439439
wbuf.putShort(k);
440440
wbuf.putByte(m);
441-
wbuf.incrementPosition(1);
441+
wbuf.incrementPosition(1); //byte 7 is unused
442442

443443
if (tgtStructure == COMPACT_EMPTY) {
444444
return bytesOut;

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
import static org.apache.datasketches.kll.KllSketch.SketchType.FLOATS_SKETCH;
3737
import static org.apache.datasketches.kll.KllSketch.SketchType.ITEMS_SKETCH;
3838

39+
import org.apache.datasketches.common.ArrayOfBooleansSerDe;
3940
import org.apache.datasketches.common.ArrayOfItemsSerDe;
4041
import org.apache.datasketches.common.Family;
4142
import org.apache.datasketches.common.SketchesArgumentException;
@@ -175,7 +176,11 @@ static int computeSketchBytes( //for COMPACT_FULL or UPDATABLE only
175176

176177
int offsetBytes = DATA_START_ADR + levelsLen * Integer.BYTES;
177178
if (sketchType == ITEMS_SKETCH) {
178-
offsetBytes += serDe.sizeOf(srcMem, offsetBytes, numItems + 2); //2 for min & max
179+
if (serDe instanceof ArrayOfBooleansSerDe) {
180+
offsetBytes += serDe.sizeOf(srcMem, offsetBytes, numItems) + 2; //2 for min & max
181+
} else {
182+
offsetBytes += serDe.sizeOf(srcMem, offsetBytes, numItems + 2); //2 for min & max
183+
}
179184
} else {
180185
final int typeBytes = sketchType.getBytes();
181186
offsetBytes += (numItems + 2) * typeBytes; //2 for min & max

src/test/java/org/apache/datasketches/common/ArrayOfXSerDeTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ public void checkBooleanItems() {
3434
WritableMemory wmem;
3535
ArrayOfBooleansSerDe serDe = new ArrayOfBooleansSerDe();
3636

37-
Boolean[] items = {true,false,true,true,false,false};
37+
Boolean[] items = {true,false,true,false,true,false,true,false,true,false};
3838
bytes = serDe.sizeOf(items);
3939
byteArr = serDe.serializeToByteArray(items);
4040
assertEquals(byteArr.length, bytes);

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

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727

2828
import java.util.Comparator;
2929

30+
import org.apache.datasketches.common.ArrayOfBooleansSerDe;
3031
import org.apache.datasketches.common.ArrayOfStringsSerDe;
3132
import org.apache.datasketches.common.SketchesArgumentException;
3233
import org.apache.datasketches.common.Util;
@@ -437,6 +438,33 @@ public void checkGetSingleItem() {
437438
assertEquals(skCompact.getSingleItem(), "1");
438439
}
439440

441+
@Test
442+
public void checkIssue484() {
443+
Boolean[] items = { true,false,true,false,true,false,true,false,true,false };
444+
KllItemsSketch<Boolean> sketch = KllItemsSketch.newHeapInstance(Boolean::compareTo, new ArrayOfBooleansSerDe());
445+
for (int i = 0; i < items.length; i++) { sketch.update(items[i]); }
446+
byte[] serialized = sketch.toByteArray();
447+
KllItemsSketch<Boolean> deserialized =
448+
KllItemsSketch.wrap(Memory.wrap(serialized), Boolean::compareTo, new ArrayOfBooleansSerDe());
449+
checkSketchesEqual(items, sketch, deserialized);
450+
}
451+
452+
private static <T> void checkSketchesEqual(T[] items, KllItemsSketch<T> expected, KllItemsSketch<T> actual) {
453+
KllItemsSketchSortedView<T> expSV = expected.getSortedView();
454+
KllItemsSketchSortedView<T> actSV = actual.getSortedView();
455+
long N = actSV.getN();
456+
long[] expCumWts = expSV.getCumulativeWeights();
457+
Boolean[] expItemsArr = (Boolean[])expSV.getQuantiles();
458+
long[] actCumWts = actSV.getCumulativeWeights();
459+
Boolean[] actItemsArr = (Boolean[])actSV.getQuantiles();
460+
printf("%3s %8s %8s\n", "i","Actual", "Expected");
461+
for (int i = 0; i < N; i++) {
462+
printf("%3d %8s %8s\n", i, actItemsArr[i].toString(), expItemsArr[i].toString());
463+
}
464+
assertEquals(actCumWts, expCumWts);
465+
assertEquals(actItemsArr, expItemsArr);
466+
}
467+
440468
private final static boolean enablePrinting = false;
441469

442470
/**

0 commit comments

Comments
 (0)