Repository: prestodb/presto
Issue: Broken full CI tests in presto-orc

Running full-ci in Jenkins turns up the following test failures. These can be reproduced in IntelliJ also.

```
testCharDirectSequence(com.facebook.presto.orc.TestFullOrcReader)  Time elapsed: 2.737 sec  <<< FAILURE!
java.lang.ArrayIndexOutOfBoundsException: 11111

testStringDirectSequence(com.facebook.presto.orc.TestFullOrcReader)  Time elapsed: 55.727 sec  <<< FAILURE!
java.lang.ArrayIndexOutOfBoundsException


Results :

Failed tests: 
  TestFullOrcReader>AbstractTestOrcReader.testCharDirectSequence:308 » ArrayIndexOutOfBounds
  TestFullOrcReader.testStringDirectSequence » ArrayIndexOutOfBounds
```

Changed files:
presto-spi/src/main/java/com/facebook/presto/spi/block/DictionaryBlockEncoding.java
presto-spi/src/main/java/com/facebook/presto/spi/block/SliceArrayBlock.java
presto-main/src/test/java/com/facebook/presto/block/TestDictionaryBlock.java
presto-spi/src/main/java/com/facebook/presto/spi/block/DictionaryBlock.java
presto-spi/src/main/java/com/facebook/presto/spi/block/AbstractInterleavedBlock.java

diff --git a/presto-main/src/test/java/com/facebook/presto/block/TestDictionaryBlock.java b/presto-main/src/test/java/com/facebook/presto/block/TestDictionaryBlock.java
index 9bbfc30219..25f4dc6d0e 100644
--- a/presto-main/src/test/java/com/facebook/presto/block/TestDictionaryBlock.java
+++ b/presto-main/src/test/java/com/facebook/presto/block/TestDictionaryBlock.java
@@ -19,7 +19,6 @@ import com.google.common.primitives.Ints;
 import io.airlift.slice.Slice;
 import org.testng.annotations.Test;
 
-import java.util.Arrays;
 import java.util.List;
 
 import static io.airlift.slice.SizeOf.SIZE_OF_INT;
@@ -55,6 +54,7 @@ public class TestDictionaryBlock
             throws Exception
     {
         Slice[] expectedValues = createExpectedValues(10);
+        Slice firstExpectedValue = expectedValues[0];
         DictionaryBlock dictionaryBlock = createDictionaryBlock(expectedValues, 100);
 
         List<Integer> positionsToCopy = Ints.asList(0, 10, 20, 30, 40);
@@ -62,7 +62,8 @@ public class TestDictionaryBlock
 
         assertEquals(copiedBlock.getDictionary().getPositionCount(), 1);
         assertEquals(copiedBlock.getPositionCount(), positionsToCopy.size());
-        assertBlock(copiedBlock.getDictionary(), Arrays.copyOfRange(expectedValues, 0, 1));
+        assertBlock(copiedBlock.getDictionary(), new Slice[]{firstExpectedValue});
+        assertBlock(copiedBlock, new Slice[]{firstExpectedValue, firstExpectedValue, firstExpectedValue, firstExpectedValue, firstExpectedValue});
     }
 
     @Test
diff --git a/presto-spi/src/main/java/com/facebook/presto/spi/block/AbstractInterleavedBlock.java b/presto-spi/src/main/java/com/facebook/presto/spi/block/AbstractInterleavedBlock.java
index ef5b973ced..0a3f465a50 100644
--- a/presto-spi/src/main/java/com/facebook/presto/spi/block/AbstractInterleavedBlock.java
+++ b/presto-spi/src/main/java/com/facebook/presto/spi/block/AbstractInterleavedBlock.java
@@ -255,6 +255,11 @@ public abstract class AbstractInterleavedBlock
     @Override
     public int getRegionSizeInBytes(int position, int length)
     {
+        if (position == 0 && length == getPositionCount()) {
+            // Calculation of getRegionSizeInBytes is expensive in this class.
+            // On the other hand, getSizeInBytes result is cached or pre-computed.
+            return getSizeInBytes();
+        }
         validateRange(position, length);
         int result = 0;
         for (int blockIndex = 0; blockIndex < getBlockCount(); blockIndex++) {
diff --git a/presto-spi/src/main/java/com/facebook/presto/spi/block/DictionaryBlock.java b/presto-spi/src/main/java/com/facebook/presto/spi/block/DictionaryBlock.java
index 38f016dd94..8b6b50b9c9 100644
--- a/presto-spi/src/main/java/com/facebook/presto/spi/block/DictionaryBlock.java
+++ b/presto-spi/src/main/java/com/facebook/presto/spi/block/DictionaryBlock.java
@@ -218,15 +218,21 @@ public class DictionaryBlock
     @Override
     public int getRegionSizeInBytes(int positionOffset, int length)
     {
+        if (positionOffset == 0 && length == getPositionCount()) {
+            // Calculation of getRegionSizeInBytes is expensive in this class.
+            // On the other hand, getSizeInBytes result is cached.
+            return getSizeInBytes();
+        }
+
         int sizeInBytes = 0;
-        boolean[] seen = new boolean[length];
-        for (int i = positionOffset; i < length; i++) {
+        boolean[] seen = new boolean[dictionary.getPositionCount()];
+        for (int i = positionOffset; i < positionOffset + length; i++) {
             int position = getId(i);
-            if (!seen[position - positionOffset]) {
+            if (!seen[position]) {
                 if (!dictionary.isNull(position)) {
                     sizeInBytes += dictionary.getRegionSizeInBytes(position, 1);
                 }
-                seen[position - positionOffset] = true;
+                seen[position] = true;
             }
         }
         return sizeInBytes + (length * Integer.BYTES);
@@ -311,6 +317,9 @@ public class DictionaryBlock
 
     public int getId(int position)
     {
+        if (position < 0 || position >= positionCount) {
+            throw new IllegalArgumentException("Invalid position " + position + " in block with " + positionCount + " positions");
+        }
         return ids[position + idsOffset];
     }
 
diff --git a/presto-spi/src/main/java/com/facebook/presto/spi/block/DictionaryBlockEncoding.java b/presto-spi/src/main/java/com/facebook/presto/spi/block/DictionaryBlockEncoding.java
index 099b3f1778..3bb8f3456b 100644
--- a/presto-spi/src/main/java/com/facebook/presto/spi/block/DictionaryBlockEncoding.java
+++ b/presto-spi/src/main/java/com/facebook/presto/spi/block/DictionaryBlockEncoding.java
@@ -81,8 +81,10 @@ public class DictionaryBlockEncoding
         long leastSignificantBits = sliceInput.readLong();
         long sequenceId = sliceInput.readLong();
 
-        // we always compact the dictionary before we send it
-        return new DictionaryBlock(positionCount, dictionaryBlock, ids, true, new DictionaryId(mostSignificantBits, leastSignificantBits, sequenceId));
+        // We always compact the dictionary before we send it. However, dictionaryBlock comes from sliceInput, which may over-retain memory.
+        // As a result, setting dictionaryIsCompacted to true is not appropriate here.
+        // TODO: fix DictionaryBlock so that dictionaryIsCompacted can be set to true when the underlying block over-retains memory.
+        return new DictionaryBlock(positionCount, dictionaryBlock, ids, false, new DictionaryId(mostSignificantBits, leastSignificantBits, sequenceId));
     }
 
     @Override
diff --git a/presto-spi/src/main/java/com/facebook/presto/spi/block/SliceArrayBlock.java b/presto-spi/src/main/java/com/facebook/presto/spi/block/SliceArrayBlock.java
index 62e9d7594b..9a9198ef17 100644
--- a/presto-spi/src/main/java/com/facebook/presto/spi/block/SliceArrayBlock.java
+++ b/presto-spi/src/main/java/com/facebook/presto/spi/block/SliceArrayBlock.java
@@ -192,6 +192,11 @@ public class SliceArrayBlock
     public int getRegionSizeInBytes(int positionOffset, int length)
     {
         int positionCount = getPositionCount();
+        if (positionOffset == 0 && length == positionCount) {
+            // Calculation of getRegionSizeInBytes is expensive in this class.
+            // On the other hand, getSizeInBytes result is pre-computed.
+            return getSizeInBytes();
+        }
         if (positionOffset < 0 || length < 0 || positionOffset + length > positionCount) {
             throw new IndexOutOfBoundsException("Invalid position " + positionOffset + " in block with " + positionCount + " positions");
         }