DRILL-6202: Deprecate usage of IndexOutOfBoundsException to re-alloc vectors
closes #1144 Project: http://git-wip-us.apache.org/repos/asf/drill/repo Commit: http://git-wip-us.apache.org/repos/asf/drill/commit/28a29033 Tree: http://git-wip-us.apache.org/repos/asf/drill/tree/28a29033 Diff: http://git-wip-us.apache.org/repos/asf/drill/diff/28a29033 Branch: refs/heads/master Commit: 28a29033c0a966b4bd65ac691a7b86f41637cae2 Parents: 087b366 Author: Vlad Rozov <[email protected]> Authored: Thu Mar 1 09:36:05 2018 -0800 Committer: Vitalii Diravka <[email protected]> Committed: Sun Apr 29 23:20:55 2018 +0300 ---------------------------------------------------------------------- .../physical/impl/common/HashTableTemplate.java | 2 - .../drill/exec/store/ResourceInputStream.java | 10 +-- .../drill/test/rowSet/test/RowSetTest.java | 2 +- .../drill/jdbc/impl/DrillResultSetImpl.java | 4 +- .../src/main/java/io/netty/buffer/DrillBuf.java | 28 ++++---- .../templates/VariableLengthVectors.java | 73 +++++++------------- .../drill/exec/vector/UntypedNullVector.java | 21 ++---- .../vector/accessor/reader/ArrayReaderImpl.java | 10 ++- .../accessor/writer/BaseScalarWriter.java | 2 +- .../vector/complex/EmptyValuePopulator.java | 4 +- 10 files changed, 59 insertions(+), 97 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/drill/blob/28a29033/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/HashTableTemplate.java ---------------------------------------------------------------------- diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/HashTableTemplate.java b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/HashTableTemplate.java index 272d782..64f8144 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/HashTableTemplate.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/HashTableTemplate.java @@ -783,8 +783,6 @@ public abstract class HashTableTemplate implements HashTable { updateBatches(); // Needed to update the value vectors in the generated code with the new incoming } catch (SchemaChangeException e) { throw new IllegalStateException("Unexpected schema change", e); - } catch(IndexOutOfBoundsException ioob) { - throw new IllegalStateException("reinit update batches", ioob); } } http://git-wip-us.apache.org/repos/asf/drill/blob/28a29033/exec/java-exec/src/main/java/org/apache/drill/exec/store/ResourceInputStream.java ---------------------------------------------------------------------- diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/store/ResourceInputStream.java b/exec/java-exec/src/main/java/org/apache/drill/exec/store/ResourceInputStream.java index ec14117..6f4ebb9 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/store/ResourceInputStream.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/store/ResourceInputStream.java @@ -24,6 +24,8 @@ import java.io.IOException; import org.apache.hadoop.fs.PositionedReadable; import org.apache.hadoop.fs.Seekable; +import com.google.common.base.Preconditions; + public class ResourceInputStream extends ByteArrayInputStream implements Seekable, PositionedReadable { static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(ResourceInputStream.class); @@ -40,12 +42,10 @@ public class ResourceInputStream extends ByteArrayInputStream implements Seekabl } public int read(long position, byte b[], int off, int len) { + Preconditions.checkNotNull(b); + Preconditions.checkPositionIndexes(off, off + len, b.length); + int start = (int) position; - if (b == null) { - throw new NullPointerException(); - } else if (off < 0 || len < 0 || len > b.length - off) { - throw new IndexOutOfBoundsException(); - } if (start >= count) { return -1; http://git-wip-us.apache.org/repos/asf/drill/blob/28a29033/exec/java-exec/src/test/java/org/apache/drill/test/rowSet/test/RowSetTest.java ---------------------------------------------------------------------- diff --git a/exec/java-exec/src/test/java/org/apache/drill/test/rowSet/test/RowSetTest.java b/exec/java-exec/src/test/java/org/apache/drill/test/rowSet/test/RowSetTest.java index d5ea3dc..f5b7b01 100644 --- a/exec/java-exec/src/test/java/org/apache/drill/test/rowSet/test/RowSetTest.java +++ b/exec/java-exec/src/test/java/org/apache/drill/test/rowSet/test/RowSetTest.java @@ -727,7 +727,7 @@ public class RowSetTest extends SubOperatorTest { writer.save(); count++; } - } catch (IndexOutOfBoundsException e) { + } catch (Exception e) { assertTrue(e.getMessage().contains("Overflow")); } writer.done(); http://git-wip-us.apache.org/repos/asf/drill/blob/28a29033/exec/jdbc/src/main/java/org/apache/drill/jdbc/impl/DrillResultSetImpl.java ---------------------------------------------------------------------- diff --git a/exec/jdbc/src/main/java/org/apache/drill/jdbc/impl/DrillResultSetImpl.java b/exec/jdbc/src/main/java/org/apache/drill/jdbc/impl/DrillResultSetImpl.java index 4dfce48..8db64b4 100644 --- a/exec/jdbc/src/main/java/org/apache/drill/jdbc/impl/DrillResultSetImpl.java +++ b/exec/jdbc/src/main/java/org/apache/drill/jdbc/impl/DrillResultSetImpl.java @@ -404,8 +404,8 @@ class DrillResultSetImpl extends AvaticaResultSet implements DrillResultSet { final Cursor.Accessor accessor; try { accessor = accessorList.get(columnIndex - 1); - } catch (IndexOutOfBoundsException e) { - throw new SQLException("invalid column ordinal: " + columnIndex); + } catch (RuntimeException e) { + throw new SQLException(e); } final ColumnMetaData metaData = columnMetaDataList.get(columnIndex - 1); // Drill returns a float (4bytes) for a SQL Float whereas Calcite would return a double (8bytes) http://git-wip-us.apache.org/repos/asf/drill/blob/28a29033/exec/memory/base/src/main/java/io/netty/buffer/DrillBuf.java ---------------------------------------------------------------------- diff --git a/exec/memory/base/src/main/java/io/netty/buffer/DrillBuf.java b/exec/memory/base/src/main/java/io/netty/buffer/DrillBuf.java index 513f50b..d62a9b6 100644 --- a/exec/memory/base/src/main/java/io/netty/buffer/DrillBuf.java +++ b/exec/memory/base/src/main/java/io/netty/buffer/DrillBuf.java @@ -766,6 +766,7 @@ public final class DrillBuf extends AbstractByteBuf implements AutoCloseable { } private final static int LOG_BYTES_PER_ROW = 10; + private static final char[] HEX_CHAR = new char[] { '0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'A', 'B', 'C', 'D', 'E', 'F' }; /** * Return the buffer's byte contents in the form of a hex dump. @@ -777,23 +778,20 @@ public final class DrillBuf extends AbstractByteBuf implements AutoCloseable { * @return A hex dump in a String. */ public String toHexString(final int start, final int length) { - final int roundedStart = (start / LOG_BYTES_PER_ROW) * LOG_BYTES_PER_ROW; - - final StringBuilder sb = new StringBuilder("buffer byte dump\n"); - int index = roundedStart; - for (int nLogged = 0; nLogged < length; nLogged += LOG_BYTES_PER_ROW) { - sb.append(String.format(" [%05d-%05d]", index, index + LOG_BYTES_PER_ROW - 1)); - for (int i = 0; i < LOG_BYTES_PER_ROW; ++i) { - try { - final byte b = getByte(index++); - sb.append(String.format(" 0x%02x", b)); - } catch (IndexOutOfBoundsException ioob) { - sb.append(" <ioob>"); - } + Preconditions.checkArgument(start >= 0); + final StringBuilder sb = new StringBuilder("buffer byte dump"); + final int end = Math.min(length, this.length - start); + for (int i = 0; i < end; i++) { + if (i % LOG_BYTES_PER_ROW == 0) { + sb.append(String.format("%n [%05d-%05d]", i + start, Math.min(i + LOG_BYTES_PER_ROW - 1, end - 1) + start)); } - sb.append('\n'); + byte b = _getByte(i + start); + sb.append(" 0x").append(HEX_CHAR[b >> 4]).append(HEX_CHAR[b & 0x0F]); } - return sb.toString(); + if (length > end) { + sb.append(String.format("%n [%05d-%05d] <ioob>", start + end, start + length)); + } + return sb.append(System.lineSeparator()).toString(); } /** http://git-wip-us.apache.org/repos/asf/drill/blob/28a29033/exec/vector/src/main/codegen/templates/VariableLengthVectors.java ---------------------------------------------------------------------- diff --git a/exec/vector/src/main/codegen/templates/VariableLengthVectors.java b/exec/vector/src/main/codegen/templates/VariableLengthVectors.java index 58f4968..876d688 100644 --- a/exec/vector/src/main/codegen/templates/VariableLengthVectors.java +++ b/exec/vector/src/main/codegen/templates/VariableLengthVectors.java @@ -581,15 +581,11 @@ public final class ${minor.class}Vector extends BaseDataValueVector implements V assert index >= 0; final int currentOffset = offsetVector.getAccessor().get(index); - offsetVector.getMutator().setSafe(index + 1, currentOffset + bytes.length); - try { - data.setBytes(currentOffset, bytes, 0, bytes.length); - } catch (IndexOutOfBoundsException e) { - while (data.capacity() < currentOffset + bytes.length) { - reAlloc(); - } - data.setBytes(currentOffset, bytes, 0, bytes.length); + while (data.capacity() < currentOffset + bytes.length) { + reAlloc(); } + offsetVector.getMutator().setSafe(index + 1, currentOffset + bytes.length); + data.setBytes(currentOffset, bytes, 0, bytes.length); } /** @@ -611,15 +607,12 @@ public final class ${minor.class}Vector extends BaseDataValueVector implements V assert index >= 0; final int currentOffset = offsetVector.getAccessor().get(index); - offsetVector.getMutator().setSafe(index + 1, currentOffset + length); - try { - data.setBytes(currentOffset, bytes, start, length); - } catch (IndexOutOfBoundsException e) { - while (data.capacity() < currentOffset + length) { - reAlloc(); - } - data.setBytes(currentOffset, bytes, start, length); + + while (data.capacity() < currentOffset + length) { + reAlloc(); } + offsetVector.getMutator().setSafe(index + 1, currentOffset + length); + data.setBytes(currentOffset, bytes, start, length); } public void setSafe(int index, byte[] bytes, int start, int length) { @@ -627,15 +620,11 @@ public final class ${minor.class}Vector extends BaseDataValueVector implements V final int currentOffset = offsetVector.getAccessor().get(index); - offsetVector.getMutator().setSafe(index + 1, currentOffset + length); - try { - data.setBytes(currentOffset, bytes, start, length); - } catch (IndexOutOfBoundsException e) { - while (data.capacity() < currentOffset + length) { - reAlloc(); - } - data.setBytes(currentOffset, bytes, start, length); + while (data.capacity() < currentOffset + length) { + reAlloc(); } + offsetVector.getMutator().setSafe(index + 1, currentOffset + length); + data.setBytes(currentOffset, bytes, start, length); } @Override @@ -651,15 +640,11 @@ public final class ${minor.class}Vector extends BaseDataValueVector implements V final int len = end - start; final int outputStart = offsetVector.data.get${(minor.javaType!type.javaType)?cap_first}(index * ${type.width}); - offsetVector.getMutator().setSafe(index+1, outputStart + len); - try{ - buffer.getBytes(start, data, outputStart, len); - } catch (IndexOutOfBoundsException e) { - while (data.capacity() < outputStart + len) { - reAlloc(); - } - buffer.getBytes(start, data, outputStart, len); + while (data.capacity() < outputStart + len) { + reAlloc(); } + offsetVector.getMutator().setSafe(index + 1, outputStart + len); + buffer.getBytes(start, data, outputStart, len); } public void setSafe(int index, Nullable${minor.class}Holder holder) { @@ -671,15 +656,11 @@ public final class ${minor.class}Vector extends BaseDataValueVector implements V final int outputStart = offsetVector.data.get${(minor.javaType!type.javaType)?cap_first}(index * ${type.width}); - try { - holder.buffer.getBytes(start, data, outputStart, len); - } catch (IndexOutOfBoundsException e) { - while (data.capacity() < outputStart + len) { - reAlloc(); - } - holder.buffer.getBytes(start, data, outputStart, len); + while (data.capacity() < outputStart + len) { + reAlloc(); } - offsetVector.getMutator().setSafe(index+1, outputStart + len); + holder.buffer.getBytes(start, data, outputStart, len); + offsetVector.getMutator().setSafe(index + 1, outputStart + len); } public void setSafe(int index, ${minor.class}Holder holder) { @@ -688,15 +669,11 @@ public final class ${minor.class}Vector extends BaseDataValueVector implements V final int len = end - start; final int outputStart = offsetVector.data.get${(minor.javaType!type.javaType)?cap_first}(index * ${type.width}); - try { - holder.buffer.getBytes(start, data, outputStart, len); - } catch (IndexOutOfBoundsException e) { - while(data.capacity() < outputStart + len) { - reAlloc(); - } - holder.buffer.getBytes(start, data, outputStart, len); + while(data.capacity() < outputStart + len) { + reAlloc(); } - offsetVector.getMutator().setSafe( index+1, outputStart + len); + holder.buffer.getBytes(start, data, outputStart, len); + offsetVector.getMutator().setSafe(index + 1, outputStart + len); } /** http://git-wip-us.apache.org/repos/asf/drill/blob/28a29033/exec/vector/src/main/java/org/apache/drill/exec/vector/UntypedNullVector.java ---------------------------------------------------------------------- diff --git a/exec/vector/src/main/java/org/apache/drill/exec/vector/UntypedNullVector.java b/exec/vector/src/main/java/org/apache/drill/exec/vector/UntypedNullVector.java index 48cae7d..e2f9f77 100644 --- a/exec/vector/src/main/java/org/apache/drill/exec/vector/UntypedNullVector.java +++ b/exec/vector/src/main/java/org/apache/drill/exec/vector/UntypedNullVector.java @@ -145,14 +145,13 @@ public final class UntypedNullVector extends BaseDataValueVector implements Fixe @Override public void splitAndTransfer(int startIndex, int length) { - checkBounds(startIndex); - checkBounds(startIndex + length - 1); + Preconditions.checkPositionIndexes(startIndex, startIndex + length, valueCount); splitAndTransferTo(startIndex, length, to); } @Override public void copyValueSafe(int fromIndex, int toIndex) { - checkBounds(fromIndex); + Preconditions.checkElementIndex(fromIndex, valueCount); to.copyFromSafe(fromIndex, toIndex, UntypedNullVector.this); } } @@ -161,12 +160,6 @@ public final class UntypedNullVector extends BaseDataValueVector implements Fixe public void copyFromSafe(int fromIndex, int thisIndex, UntypedNullVector from) { } - private void checkBounds(int index) { - if (index < 0 || index >= valueCount) { - throw new IndexOutOfBoundsException(String.format( - "index: %d, expected: range(0, %d-1))", index, valueCount)); - } - } @Override public void copyEntry(int toIndex, ValueVector from, int fromIndex) { ((UntypedNullVector) from).data.getBytes(fromIndex * 4, data, toIndex * 4, 4); @@ -180,23 +173,23 @@ public final class UntypedNullVector extends BaseDataValueVector implements Fixe @Override public boolean isNull(int index){ - checkBounds(index); + Preconditions.checkElementIndex(index, valueCount); return true; } public int isSet(int index) { - checkBounds(index); + Preconditions.checkElementIndex(index, valueCount); return 0; } @Override public Object getObject(int index) { - checkBounds(index); + Preconditions.checkElementIndex(index, valueCount); return null; } public void get(int index, UntypedNullHolder holder) { - checkBounds(index); + Preconditions.checkElementIndex(index, valueCount); } } @@ -247,4 +240,4 @@ public final class UntypedNullVector extends BaseDataValueVector implements Fixe UntypedNullVector.this.valueCount = valueCount; } } -} \ No newline at end of file +} http://git-wip-us.apache.org/repos/asf/drill/blob/28a29033/exec/vector/src/main/java/org/apache/drill/exec/vector/accessor/reader/ArrayReaderImpl.java ---------------------------------------------------------------------- diff --git a/exec/vector/src/main/java/org/apache/drill/exec/vector/accessor/reader/ArrayReaderImpl.java b/exec/vector/src/main/java/org/apache/drill/exec/vector/accessor/reader/ArrayReaderImpl.java index 7f2bf39..6aa4547 100644 --- a/exec/vector/src/main/java/org/apache/drill/exec/vector/accessor/reader/ArrayReaderImpl.java +++ b/exec/vector/src/main/java/org/apache/drill/exec/vector/accessor/reader/ArrayReaderImpl.java @@ -29,6 +29,8 @@ import org.apache.drill.exec.vector.accessor.ObjectType; import org.apache.drill.exec.vector.accessor.ScalarReader; import org.apache.drill.exec.vector.accessor.TupleReader; +import com.google.common.base.Preconditions; + /** * Reader for an array-valued column. This reader provides access to specific * array members via an array index. This class implements all arrays. The @@ -132,9 +134,7 @@ public class ArrayReaderImpl implements ArrayReader, ReaderEvents { @Override public int offset() { - if (position < 0 || length <= position) { - throw new IndexOutOfBoundsException("Index = " + position + ", length = " + length); - } + Preconditions.checkElementIndex(position, length); return startOffset + position; } @@ -154,9 +154,7 @@ public class ArrayReaderImpl implements ArrayReader, ReaderEvents { */ public void set(int index) { - if (index < 0 || length < index) { - throw new IndexOutOfBoundsException("Index = " + index + ", length = " + length); - } + Preconditions.checkPositionIndex(index, length); position = index; } http://git-wip-us.apache.org/repos/asf/drill/blob/28a29033/exec/vector/src/main/java/org/apache/drill/exec/vector/accessor/writer/BaseScalarWriter.java ---------------------------------------------------------------------- diff --git a/exec/vector/src/main/java/org/apache/drill/exec/vector/accessor/writer/BaseScalarWriter.java b/exec/vector/src/main/java/org/apache/drill/exec/vector/accessor/writer/BaseScalarWriter.java index 10a2cf3..7e12149 100644 --- a/exec/vector/src/main/java/org/apache/drill/exec/vector/accessor/writer/BaseScalarWriter.java +++ b/exec/vector/src/main/java/org/apache/drill/exec/vector/accessor/writer/BaseScalarWriter.java @@ -194,7 +194,7 @@ public abstract class BaseScalarWriter extends AbstractScalarWriter { protected void overflowed() { if (listener == null) { - throw new IndexOutOfBoundsException("Overflow not supported"); + throw new UnsupportedOperationException("Overflow not supported"); } else { listener.overflowed(this); } http://git-wip-us.apache.org/repos/asf/drill/blob/28a29033/exec/vector/src/main/java/org/apache/drill/exec/vector/complex/EmptyValuePopulator.java ---------------------------------------------------------------------- diff --git a/exec/vector/src/main/java/org/apache/drill/exec/vector/complex/EmptyValuePopulator.java b/exec/vector/src/main/java/org/apache/drill/exec/vector/complex/EmptyValuePopulator.java index 4109859..2c51422 100644 --- a/exec/vector/src/main/java/org/apache/drill/exec/vector/complex/EmptyValuePopulator.java +++ b/exec/vector/src/main/java/org/apache/drill/exec/vector/complex/EmptyValuePopulator.java @@ -37,9 +37,7 @@ public class EmptyValuePopulator { * @throws java.lang.IndexOutOfBoundsException if lastIndex is negative or greater than offsets capacity. */ public void populate(int lastIndex) { - if (lastIndex < 0) { - throw new IndexOutOfBoundsException("index cannot be negative"); - } + Preconditions.checkElementIndex(lastIndex, Integer.MAX_VALUE); final UInt4Vector.Accessor accessor = offsets.getAccessor(); final UInt4Vector.Mutator mutator = offsets.getMutator(); final int lastSet = Math.max(accessor.getValueCount() - 1, 0);
