Fix IllegalStateException in TwoValuesContainer triggered from Index.…

…insert(Chunk). Fixes deephaven#652.
jcferretti · May 21, 2021 · 4c6b89f · 4c6b89f
1 parent 1870e24
commit 4c6b89f
Show file tree

Hide file tree

Showing 3 changed files with 28 additions and 11 deletions.
diff --git a/DB/src/main/java/io/deephaven/db/v2/utils/RspBitmapSequentialBuilder.java b/DB/src/main/java/io/deephaven/db/v2/utils/RspBitmapSequentialBuilder.java
@@ -89,6 +89,9 @@ public void appendRange(final long start, final long end) {
 
     @Override
     public void appendTreeIndexImpl(final long shiftAmount, final TreeIndexImpl ix, final boolean acquire) {
+        if (ix.ixIsEmpty()) {
+            return;
+        }
         if (!(ix instanceof RspBitmap) || rb == null) {
             ix.ixForEachLongRange((final long start, final long end) -> {
                 appendRange(start + shiftAmount, end + shiftAmount);

diff --git a/DB/src/main/java/io/deephaven/db/v2/utils/rsp/RspBitmap.java b/DB/src/main/java/io/deephaven/db/v2/utils/rsp/RspBitmap.java
@@ -244,20 +244,22 @@ private Container createOrUpdateContainerForValues(@NotNull final LongChunk<Orde
                 if (firstValue == key) {
                     return null;
                 }
-                if (firstValue + 1 < key) {
-                    final short keyLowBits = lowBitsAsShort(key);
-                    final short firstValueLowBits = lowBitsAsShort(firstValue);
-                    return new TwoValuesContainer(firstValueLowBits, keyLowBits);
+                final long left, right;
+                if (firstValue < key) {
+                    left = firstValue;
+                    right = key;
+                } else {
+                    left = key;
+                    right = firstValue;
                 }
-                if (firstValue + 1 == key) {
-                    final int start = lowBitsAsInt(firstValue);
-                    final int end = lowBitsAsInt(key);
+                if (left + 1 == right) {
+                    final int start = lowBitsAsInt(left);
+                    final int end = lowBitsAsInt(right);
                     return new SingleRangeContainer(start, end + 1);
                 }
-                // firstValue + 1 > key
-                final short keyLowBits = lowBitsAsShort(key);
-                final short firstValueLowBits = lowBitsAsShort(firstValue);
-                return new TwoValuesContainer(keyLowBits, firstValueLowBits);
+                final short leftLow = lowBitsAsShort(left);
+                final short rightLow = lowBitsAsShort(right);
+                return new TwoValuesContainer(leftLow, rightLow);
             }
             final short firstValueLowBits = lowBitsAsShort(firstValue);
             return container.iset(firstValueLowBits);

diff --git a/DB/src/test/java/io/deephaven/db/v2/utils/rsp/RspBitmapTest.java b/DB/src/test/java/io/deephaven/db/v2/utils/rsp/RspBitmapTest.java
@@ -4209,4 +4209,16 @@ public void testInvertNoAccForCoverage() {
         assertEquals(3, result.get(2));
         assertEquals(rb.getCardinality() - 1, result.get(picks.getCardinality() - 1));
     }
+
+    @Test
+    public void testAddValuesUnsafeRegression() {
+        RspBitmap rb = new RspBitmap();
+        rb = rb.add(3);
+        final WritableLongChunk<OrderedKeyIndices> chunk = WritableLongChunk.makeWritableChunk(4);
+        chunk.setSize(0);
+        chunk.add(4);
+        chunk.add(BLOCK_SIZE + 10);
+        // we saw: "java.lang.IllegalStateException: iv1=3, iv2=4"
+        rb.addValuesUnsafeNoWriteCheck(chunk, 0, 2);
+    }
 }