This is an automated email from the ASF dual-hosted git repository. sorabh pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/drill.git
commit 56f951cc7a03e497ba34eeca4bd9265ae30c4650 Author: Salim Achouche <[email protected]> AuthorDate: Mon Jul 2 22:04:20 2018 -0700 DRILL-6579: Added sanity checks to the Parquet reader to avoid infinite loops closes #1361 --- .../drill/exec/store/parquet/columnreaders/BatchReader.java | 4 ++-- .../exec/store/parquet/columnreaders/VarLenBinaryReader.java | 3 ++- .../exec/store/parquet/columnreaders/VarLenBulkPageReader.java | 7 +++++-- .../store/parquet/columnreaders/VarLenEntryDictionaryReader.java | 3 +++ .../exec/store/parquet/columnreaders/VarLenEntryReader.java | 3 +++ .../exec/store/parquet/columnreaders/VarLenFixedEntryReader.java | 9 +++++---- .../parquet/columnreaders/VarLenNullableDictionaryReader.java | 3 +++ .../store/parquet/columnreaders/VarLenNullableEntryReader.java | 3 +++ .../parquet/columnreaders/VarLenNullableFixedEntryReader.java | 6 ++++-- .../exec/store/parquet/columnreaders/VarLenOverflowReader.java | 3 +++ 10 files changed, 33 insertions(+), 11 deletions(-) diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/BatchReader.java b/exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/BatchReader.java index 25dfbc8..f5825e4 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/BatchReader.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/BatchReader.java @@ -40,9 +40,9 @@ public abstract class BatchReader { ColumnReader<?> firstColumnStatus = readState.getFirstColumnReader(); int currBatchNumRecords = readState.batchSizerMgr().getCurrentRecordsPerBatch(); long recordsToRead = Math.min(currBatchNumRecords, readState.getRemainingValuesToRead()); - int readCount = readRecords(firstColumnStatus, recordsToRead); - + int readCount = recordsToRead > 0 ? readRecords(firstColumnStatus, recordsToRead) : 0; readState.fillNullVectors(readCount); + return readCount; } diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/VarLenBinaryReader.java b/exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/VarLenBinaryReader.java index 1fb224d..cba2a79 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/VarLenBinaryReader.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/VarLenBinaryReader.java @@ -18,6 +18,7 @@ package org.apache.drill.exec.store.parquet.columnreaders; import com.google.common.base.Stopwatch; +import com.google.common.base.Preconditions; import com.google.common.collect.Lists; import java.io.IOException; import java.util.ArrayList; @@ -111,7 +112,7 @@ public class VarLenBinaryReader { // Read the column data int readColumns = columnReader.readRecordsInBulk(batchNumRecords); - assert readColumns <= batchNumRecords : "Reader cannot return more values than requested.."; + Preconditions.checkState(readColumns <= batchNumRecords, "Reader cannot return more values than requested.."); if (!overflowCondition) { if (prevReadColumns >= 0 && prevReadColumns != readColumns) { diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/VarLenBulkPageReader.java b/exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/VarLenBulkPageReader.java index 0e50406..81b7264 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/VarLenBulkPageReader.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/VarLenBulkPageReader.java @@ -17,6 +17,7 @@ */ package org.apache.drill.exec.store.parquet.columnreaders; +import com.google.common.base.Preconditions; import java.io.IOException; import java.nio.ByteBuffer; import java.nio.ByteOrder; @@ -106,11 +107,13 @@ final class VarLenBulkPageReader { pageInfo.dictionaryValueReader = pageInfoInput.dictionaryValueReader; pageInfo.numPageValues = pageInfoInput.numPageValues; if (clear) { - buffer.clear(); - } + buffer.clear(); + } } final VarLenColumnBulkEntry getEntry(int valuesToRead) { + Preconditions.checkArgument(valuesToRead > 0, "Number of values to read [%s] should be greater than zero", valuesToRead); + VarLenColumnBulkEntry entry = null; // If there is overflow data, then we need to consume it first diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/VarLenEntryDictionaryReader.java b/exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/VarLenEntryDictionaryReader.java index 8ba7ac4..7d76263 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/VarLenEntryDictionaryReader.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/VarLenEntryDictionaryReader.java @@ -17,6 +17,7 @@ */ package org.apache.drill.exec.store.parquet.columnreaders; +import com.google.common.base.Preconditions; import java.nio.ByteBuffer; import org.apache.drill.exec.store.parquet.columnreaders.VarLenColumnBulkInput.ColumnPrecisionInfo; import org.apache.drill.exec.store.parquet.columnreaders.VarLenColumnBulkInput.DictionaryReaderWrapper; @@ -50,6 +51,8 @@ final class VarLenEntryDictionaryReader extends VarLenAbstractPageEntryReader { final DictionaryReaderWrapper valueReader = pageInfo.dictionaryValueReader; final int[] valueLengths = entry.getValuesLength(); final int readBatch = Math.min(entry.getMaxEntries(), valuesToRead); + Preconditions.checkState(readBatch > 0, "Read batch count [%s] should be greater than zero", readBatch); + final byte[] tgtBuff = entry.getInternalDataArray(); final int tgtLen = tgtBuff.length; diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/VarLenEntryReader.java b/exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/VarLenEntryReader.java index d95050d..cec0c7f 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/VarLenEntryReader.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/VarLenEntryReader.java @@ -17,6 +17,7 @@ */ package org.apache.drill.exec.store.parquet.columnreaders; +import com.google.common.base.Preconditions; import java.nio.ByteBuffer; import org.apache.drill.common.exceptions.DrillRuntimeException; import org.apache.drill.exec.store.parquet.columnreaders.VarLenColumnBulkInput.ColumnPrecisionInfo; @@ -51,6 +52,8 @@ final class VarLenEntryReader extends VarLenAbstractPageEntryReader { final int[] valueLengths = entry.getValuesLength(); final int readBatch = Math.min(entry.getMaxEntries(), valuesToRead); + Preconditions.checkState(readBatch > 0, "Read batch count [%s] should be greater than zero", readBatch); + final byte[] tgtBuff = entry.getInternalDataArray(); final byte[] srcBuff = buffer.array(); final int srcLen = buffer.remaining(); diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/VarLenFixedEntryReader.java b/exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/VarLenFixedEntryReader.java index e8dc15f..a6e7077 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/VarLenFixedEntryReader.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/VarLenFixedEntryReader.java @@ -17,6 +17,7 @@ */ package org.apache.drill.exec.store.parquet.columnreaders; +import com.google.common.base.Preconditions; import java.nio.ByteBuffer; import org.apache.drill.exec.store.parquet.columnreaders.VarLenColumnBulkInput.ColumnPrecisionInfo; import org.apache.drill.exec.store.parquet.columnreaders.VarLenColumnBulkInput.PageDataInfo; @@ -32,19 +33,19 @@ final class VarLenFixedEntryReader extends VarLenAbstractPageEntryReader { VarLenColumnBulkInputCallback containerCallback) { super(buffer, pageInfo, columnPrecInfo, entry, containerCallback); + Preconditions.checkArgument(columnPrecInfo.precision >= 0, "Fixed length precision [%s] cannot be lower than zero", columnPrecInfo.precision); } /** {@inheritDoc} */ @Override final VarLenColumnBulkEntry getEntry(int valuesToRead) { - assert columnPrecInfo.precision >= 0 : "Fixed length precision cannot be lower than zero"; - load(true); // load new data to process final int expectedDataLen = columnPrecInfo.precision; final int entrySz = 4 + columnPrecInfo.precision; - final int maxValues = Math.min(entry.getMaxEntries(), (pageInfo.pageDataLen - pageInfo.pageDataOff) / entrySz); - final int readBatch = Math.min(maxValues, valuesToRead); + final int readBatch = Math.min(entry.getMaxEntries(), valuesToRead); + Preconditions.checkState(readBatch > 0, "Read batch count [%d] should be greater than zero", readBatch); + final int[] valueLengths = entry.getValuesLength(); final byte[] tgtBuff = entry.getInternalDataArray(); final byte[] srcBuff = buffer.array(); diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/VarLenNullableDictionaryReader.java b/exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/VarLenNullableDictionaryReader.java index f7b6dce..e33919f 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/VarLenNullableDictionaryReader.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/VarLenNullableDictionaryReader.java @@ -17,6 +17,7 @@ */ package org.apache.drill.exec.store.parquet.columnreaders; +import com.google.common.base.Preconditions; import java.nio.ByteBuffer; import org.apache.drill.exec.store.parquet.columnreaders.VarLenColumnBulkInput.ColumnPrecisionInfo; import org.apache.drill.exec.store.parquet.columnreaders.VarLenColumnBulkInput.DictionaryReaderWrapper; @@ -52,6 +53,8 @@ final class VarLenNullableDictionaryReader extends VarLenAbstractPageEntryReader final DictionaryReaderWrapper valueReader = pageInfo.dictionaryValueReader; final int[] valueLengths = entry.getValuesLength(); final int readBatch = Math.min(entry.getMaxEntries(), valuesToRead); + Preconditions.checkState(readBatch > 0, "Read batch count [%s] should be greater than zero", readBatch); + final byte[] tgtBuff = entry.getInternalDataArray(); final int tgtLen = tgtBuff.length; diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/VarLenNullableEntryReader.java b/exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/VarLenNullableEntryReader.java index 7ffb27a..ce39859 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/VarLenNullableEntryReader.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/VarLenNullableEntryReader.java @@ -17,6 +17,7 @@ */ package org.apache.drill.exec.store.parquet.columnreaders; +import com.google.common.base.Preconditions; import java.nio.ByteBuffer; import org.apache.drill.common.exceptions.DrillRuntimeException; import org.apache.drill.exec.store.parquet.columnreaders.VarLenColumnBulkInput.ColumnPrecisionInfo; @@ -53,6 +54,8 @@ final class VarLenNullableEntryReader extends VarLenAbstractPageEntryReader { final int[] valueLengths = entry.getValuesLength(); final int readBatch = Math.min(entry.getMaxEntries(), valuesToRead); + Preconditions.checkState(readBatch > 0, "Read batch count [%s] should be greater than zero", readBatch); + final byte[] tgtBuff = entry.getInternalDataArray(); final byte[] srcBuff = buffer.array(); final int srcLen = buffer.remaining(); diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/VarLenNullableFixedEntryReader.java b/exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/VarLenNullableFixedEntryReader.java index 98089fd..3869113 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/VarLenNullableFixedEntryReader.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/VarLenNullableFixedEntryReader.java @@ -17,6 +17,7 @@ */ package org.apache.drill.exec.store.parquet.columnreaders; +import com.google.common.base.Preconditions; import java.nio.ByteBuffer; import org.apache.drill.exec.store.parquet.columnreaders.VarLenColumnBulkInput.ColumnPrecisionInfo; import org.apache.drill.exec.store.parquet.columnreaders.VarLenColumnBulkInput.PageDataInfo; @@ -33,19 +34,20 @@ final class VarLenNullableFixedEntryReader extends VarLenAbstractPageEntryReader VarLenColumnBulkInputCallback containerCallback) { super(buffer, pageInfo, columnPrecInfo, entry, containerCallback); + Preconditions.checkArgument(columnPrecInfo.precision >= 0, "Fixed length precision cannot be lower than zero"); } /** {@inheritDoc} */ @Override final VarLenColumnBulkEntry getEntry(int valuesToRead) { - assert columnPrecInfo.precision >= 0 : "Fixed length precision cannot be lower than zero"; - // TODO - We should not use force reload for sparse columns (values with lot of nulls) load(true); // load new data to process final int expectedDataLen = columnPrecInfo.precision; final int entrySz = 4 + columnPrecInfo.precision; final int readBatch = Math.min(entry.getMaxEntries(), valuesToRead); + Preconditions.checkState(readBatch > 0, "Read batch count [%s] should be greater than zero", readBatch); + final int[] valueLengths = entry.getValuesLength(); final byte[] tgtBuff = entry.getInternalDataArray(); final byte[] srcBuff = buffer.array(); diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/VarLenOverflowReader.java b/exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/VarLenOverflowReader.java index cacd5c8..6c8891f 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/VarLenOverflowReader.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/VarLenOverflowReader.java @@ -17,6 +17,7 @@ */ package org.apache.drill.exec.store.parquet.columnreaders; +import com.google.common.base.Preconditions; import java.nio.ByteBuffer; import org.apache.drill.exec.store.parquet.columnreaders.VarLenColumnBulkInput.VarLenColumnBulkInputCallback; @@ -80,6 +81,8 @@ public final class VarLenOverflowReader extends VarLenAbstractEntryReader { // load some overflow data for processing final int maxValues = Math.min(entry.getMaxEntries(), valuesToRead); final int numAvailableValues = overflowDataCache.load(overflowState.currValueIdx, maxValues); + Preconditions.checkState(numAvailableValues > 0, "Number values to read [%s] should be greater than zero", numAvailableValues); + final int firstValueDataOffset = getDataBufferStartOffset() + adjustDataOffset(overflowState.currValueIdx); int totalDataLen = 0; int currValueIdx = overflowState.currValueIdx;
