DRILL-1702: Fix issue where 4 byte DrillBuf is sent even though there are no values when using varchar or repeated vector.
Project: http://git-wip-us.apache.org/repos/asf/incubator-drill/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-drill/commit/116f6d16 Tree: http://git-wip-us.apache.org/repos/asf/incubator-drill/tree/116f6d16 Diff: http://git-wip-us.apache.org/repos/asf/incubator-drill/diff/116f6d16 Branch: refs/heads/master Commit: 116f6d167f77289f1b03e8e0895851a0f1f411eb Parents: 5ad6774 Author: Jacques Nadeau <jacq...@apache.org> Authored: Wed Nov 12 22:02:50 2014 -0800 Committer: Jacques Nadeau <jacq...@apache.org> Committed: Thu Nov 13 09:17:27 2014 -0800 ---------------------------------------------------------------------- .../main/codegen/templates/RepeatedValueVectors.java | 7 +++++-- .../codegen/templates/VariableLengthVectors.java | 2 +- .../apache/drill/exec/record/RecordBatchLoader.java | 3 +++ .../exec/store/parquet2/DrillParquetReader.java | 3 +++ .../java/org/apache/drill/exec/util/VectorUtil.java | 7 ++++++- .../apache/drill/exec/vector/complex/MapVector.java | 3 +++ .../drill/exec/vector/complex/RepeatedMapVector.java | 15 ++++++++++++--- 7 files changed, 33 insertions(+), 7 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/116f6d16/exec/java-exec/src/main/codegen/templates/RepeatedValueVectors.java ---------------------------------------------------------------------- diff --git a/exec/java-exec/src/main/codegen/templates/RepeatedValueVectors.java b/exec/java-exec/src/main/codegen/templates/RepeatedValueVectors.java index 0f86db9..bf93d00 100644 --- a/exec/java-exec/src/main/codegen/templates/RepeatedValueVectors.java +++ b/exec/java-exec/src/main/codegen/templates/RepeatedValueVectors.java @@ -77,6 +77,9 @@ public final class Repeated${minor.class}Vector extends BaseValueVector implemen } public int getBufferSize(){ + if(accessor.getGroupCount() == 0){ + return 0; + } return offsets.getBufferSize() + values.getBufferSize(); } @@ -411,7 +414,7 @@ public final class Repeated${minor.class}Vector extends BaseValueVector implemen Repeated${minor.class}Vector.this.parentValueCount = parentValueCount; Repeated${minor.class}Vector.this.childValueCount = childValueCount; values.getMutator().setValueCount(childValueCount); - offsets.getMutator().setValueCount(childValueCount + 1); + offsets.getMutator().setValueCount(parentValueCount == 0 ? 0 : parentValueCount + 1); } public boolean startNewGroup(int index) { @@ -512,7 +515,7 @@ public final class Repeated${minor.class}Vector extends BaseValueVector implemen public void setValueCount(int groupCount) { parentValueCount = groupCount; childValueCount = offsets.getAccessor().get(groupCount); - offsets.getMutator().setValueCount(groupCount+1); + offsets.getMutator().setValueCount(groupCount == 0 ? 0 : groupCount+1); values.getMutator().setValueCount(childValueCount); } http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/116f6d16/exec/java-exec/src/main/codegen/templates/VariableLengthVectors.java ---------------------------------------------------------------------- diff --git a/exec/java-exec/src/main/codegen/templates/VariableLengthVectors.java b/exec/java-exec/src/main/codegen/templates/VariableLengthVectors.java index 5338a71..b8ffe5d 100644 --- a/exec/java-exec/src/main/codegen/templates/VariableLengthVectors.java +++ b/exec/java-exec/src/main/codegen/templates/VariableLengthVectors.java @@ -559,7 +559,7 @@ public final class ${minor.class}Vector extends BaseDataValueVector implements V allocationMonitor = 0; } VectorTrimmer.trim(data, idx); - offsetVector.getMutator().setValueCount(valueCount+1); + offsetVector.getMutator().setValueCount(valueCount == 0 ? 0 : valueCount+1); } @Override http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/116f6d16/exec/java-exec/src/main/java/org/apache/drill/exec/record/RecordBatchLoader.java ---------------------------------------------------------------------- diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/record/RecordBatchLoader.java b/exec/java-exec/src/main/java/org/apache/drill/exec/record/RecordBatchLoader.java index 8a12cbd..97756e2 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/record/RecordBatchLoader.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/record/RecordBatchLoader.java @@ -32,6 +32,7 @@ import org.apache.drill.exec.proto.UserBitShared.SerializedField; import org.apache.drill.exec.vector.AllocationHelper; import org.apache.drill.exec.vector.ValueVector; +import com.google.common.base.Preconditions; import com.google.common.collect.Maps; public class RecordBatchLoader implements VectorAccessible, Iterable<VectorWrapper<?>>{ @@ -94,6 +95,8 @@ public class RecordBatchLoader implements VectorAccessible, Iterable<VectorWrapp newVectors.add(v); } + Preconditions.checkArgument(buf == null || bufOffset == buf.capacity()); + if(!oldFields.isEmpty()){ schemaChanged = true; for(ValueVector v : oldFields.values()){ http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/116f6d16/exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet2/DrillParquetReader.java ---------------------------------------------------------------------- diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet2/DrillParquetReader.java b/exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet2/DrillParquetReader.java index 8765935..c3e8330 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet2/DrillParquetReader.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet2/DrillParquetReader.java @@ -214,6 +214,9 @@ public class DrillParquetReader extends AbstractRecordReader { totalRead++; if (count % fillLevelCheckFrequency == 0) { if (getPercentFilled() > fillLevelCheckThreshold) { + if(!recordMaterializer.ok()){ + throw new RuntimeException(String.format("The setting for `%s` is too high for your Parquet records. Please set a lower check threshold and retry your query. ", ExecConstants.PARQUET_VECTOR_FILL_CHECK_THRESHOLD)); + } break; } } http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/116f6d16/exec/java-exec/src/main/java/org/apache/drill/exec/util/VectorUtil.java ---------------------------------------------------------------------- diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/util/VectorUtil.java b/exec/java-exec/src/main/java/org/apache/drill/exec/util/VectorUtil.java index c0fb572..9919de1 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/util/VectorUtil.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/util/VectorUtil.java @@ -48,7 +48,12 @@ public class VectorUtil { int columnCounter = 0; for (VectorWrapper<?> vw : va) { boolean lastColumn = columnCounter == width - 1; - Object o = vw.getValueVector().getAccessor().getObject(row); + Object o ; + try{ + o = vw.getValueVector().getAccessor().getObject(row); + }catch(Exception e){ + throw new RuntimeException("failure while trying to read column " + vw.getField().getPath().toExpr()); + } if (o == null) { //null value String value = "null"; http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/116f6d16/exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/MapVector.java ---------------------------------------------------------------------- diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/MapVector.java b/exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/MapVector.java index 7a0afdb..8fb56e6 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/MapVector.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/MapVector.java @@ -19,6 +19,7 @@ package org.apache.drill.exec.vector.complex; import com.google.common.collect.Ordering; import com.google.common.primitives.Ints; + import io.netty.buffer.DrillBuf; import java.util.HashMap; @@ -342,6 +343,8 @@ public class MapVector extends AbstractContainerVector { } bufOffset += fmd.getBufferLength(); } + + Preconditions.checkArgument(bufOffset == buf.capacity()); } @Override http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/116f6d16/exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/RepeatedMapVector.java ---------------------------------------------------------------------- diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/RepeatedMapVector.java b/exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/RepeatedMapVector.java index 97f37e2..1b01c55 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/RepeatedMapVector.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/RepeatedMapVector.java @@ -143,7 +143,7 @@ public class RepeatedMapVector extends AbstractContainerVector implements Repeat @Override public int getBufferSize() { - if (accessor.getValueCount() == 0 || vectors.isEmpty()) { + if (accessor.getGroupCount() == 0) { return 0; } long buffer = offsets.getBufferSize(); @@ -430,6 +430,7 @@ public class RepeatedMapVector extends AbstractContainerVector implements Repeat @Override public DrillBuf[] getBuffers(boolean clear) { + int bufSize = getBufferSize(); List<DrillBuf> bufs = Lists.newArrayList(offsets.getBuffers(clear)); for (ValueVector v : vectors.values()) { @@ -437,6 +438,12 @@ public class RepeatedMapVector extends AbstractContainerVector implements Repeat bufs.add(b); } } + int actualBufSize = 0 ; + for(DrillBuf b : bufs){ + actualBufSize += b.writerIndex(); + } + + Preconditions.checkArgument(actualBufSize == bufSize); return bufs.toArray(new DrillBuf[bufs.size()]); } @@ -462,6 +469,8 @@ public class RepeatedMapVector extends AbstractContainerVector implements Repeat } bufOffset += fmd.getBufferLength(); } + + Preconditions.checkArgument(bufOffset == buf.capacity()); } @Override @@ -471,7 +480,7 @@ public class RepeatedMapVector extends AbstractContainerVector implements Repeat .setBufferLength(getBufferSize()) // .setGroupCount(accessor.getGroupCount()) // while we don't need to actually read this on load, we need it to make sure we don't skip deserialization of this vector - .setValueCount(accessor.getValueCount()); + .setValueCount(accessor.getGroupCount()); for (ValueVector v : vectors.values()) { b.addChild(v.getMetadata()); } @@ -600,7 +609,7 @@ public class RepeatedMapVector extends AbstractContainerVector implements Repeat public void setValueCount(int topLevelValueCount) { populateEmpties(topLevelValueCount); - offsets.getMutator().setValueCount(topLevelValueCount+1); + offsets.getMutator().setValueCount(topLevelValueCount == 0 ? 0 : topLevelValueCount+1); int childValueCount = offsets.getAccessor().get(topLevelValueCount); for (ValueVector v : vectors.values()) { v.getMutator().setValueCount(childValueCount);
