You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@drill.apache.org by ve...@apache.org on 2015/05/11 04:07:55 UTC
drill git commit: DRILL-3017: Safeguard against NPEs in
RecordReader.cleanup()s
Repository: drill
Updated Branches:
refs/heads/master 6076cc643 -> 826fc5b9c
DRILL-3017: Safeguard against NPEs in RecordReader.cleanup()s
Project: http://git-wip-us.apache.org/repos/asf/drill/repo
Commit: http://git-wip-us.apache.org/repos/asf/drill/commit/826fc5b9
Tree: http://git-wip-us.apache.org/repos/asf/drill/tree/826fc5b9
Diff: http://git-wip-us.apache.org/repos/asf/drill/diff/826fc5b9
Branch: refs/heads/master
Commit: 826fc5b9c2a6f044101967a9a2e49b20af2dae76
Parents: 6076cc6
Author: vkorukanti <ve...@gmail.com>
Authored: Sun May 10 15:31:58 2015 -0700
Committer: vkorukanti <ve...@gmail.com>
Committed: Sun May 10 15:31:58 2015 -0700
----------------------------------------------------------------------
.../drill/exec/store/hive/HiveRecordReader.java | 5 +++-
.../compliant/CompliantTextRecordReader.java | 5 +++-
.../columnreaders/ParquetRecordReader.java | 19 ++++++++++------
.../exec/store/parquet2/DrillParquetReader.java | 5 +++-
.../exec/store/text/DrillTextRecordReader.java | 5 +++-
.../java/org/apache/drill/BaseTestQuery.java | 24 +++++++++++++++++++-
.../apache/drill/TestDisabledFunctionality.java | 11 +++++----
7 files changed, 58 insertions(+), 16 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/drill/blob/826fc5b9/contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveRecordReader.java
----------------------------------------------------------------------
diff --git a/contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveRecordReader.java b/contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveRecordReader.java
index 5394ee3..3c8b9ba 100644
--- a/contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveRecordReader.java
+++ b/contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveRecordReader.java
@@ -344,7 +344,10 @@ public class HiveRecordReader extends AbstractRecordReader {
@Override
public void cleanup() {
try {
- reader.close();
+ if (reader != null) {
+ reader.close();
+ reader = null;
+ }
} catch (Exception e) {
logger.warn("Failure while closing Hive Record reader.", e);
}
http://git-wip-us.apache.org/repos/asf/drill/blob/826fc5b9/exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/text/compliant/CompliantTextRecordReader.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/text/compliant/CompliantTextRecordReader.java b/exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/text/compliant/CompliantTextRecordReader.java
index b2af32d..254e0d8 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/text/compliant/CompliantTextRecordReader.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/text/compliant/CompliantTextRecordReader.java
@@ -144,7 +144,10 @@ public class CompliantTextRecordReader extends AbstractRecordReader {
@Override
public void cleanup() {
try {
- reader.close();
+ if (reader != null) {
+ reader.close();
+ reader = null;
+ }
} catch (IOException e) {
logger.warn("Exception while closing stream.", e);
}
http://git-wip-us.apache.org/repos/asf/drill/blob/826fc5b9/exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/ParquetRecordReader.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/ParquetRecordReader.java b/exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/ParquetRecordReader.java
index 58cf321..2f07fb3 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/ParquetRecordReader.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/ParquetRecordReader.java
@@ -455,17 +455,22 @@ public class ParquetRecordReader extends AbstractRecordReader {
// enable this for debugging when it is know that a whole file will be read
// limit kills upstream operators once it has enough records, so this assert will fail
// assert totalRecordsRead == footer.getBlocks().get(rowGroupIndex).getRowCount();
- for (ColumnReader column : columnStatuses) {
- column.clear();
+ if (columnStatuses != null) {
+ for (ColumnReader column : columnStatuses) {
+ column.clear();
+ }
+ columnStatuses.clear();
+ columnStatuses = null;
}
- columnStatuses.clear();
codecFactory.close();
- for (VarLengthColumn r : varLengthReader.columns) {
- r.clear();
+ if (varLengthReader != null) {
+ for (VarLengthColumn r : varLengthReader.columns) {
+ r.clear();
+ }
+ varLengthReader.columns.clear();
+ varLengthReader = null;
}
- varLengthReader.columns.clear();
}
-
}
http://git-wip-us.apache.org/repos/asf/drill/blob/826fc5b9/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 99ac19c..4e7d628 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
@@ -342,7 +342,10 @@ public class DrillParquetReader extends AbstractRecordReader {
@Override
public void cleanup() {
try {
- pageReadStore.close();
+ if (pageReadStore != null) {
+ pageReadStore.close();
+ pageReadStore = null;
+ }
} catch (IOException e) {
logger.warn("Failure while closing PageReadStore", e);
}
http://git-wip-us.apache.org/repos/asf/drill/blob/826fc5b9/exec/java-exec/src/main/java/org/apache/drill/exec/store/text/DrillTextRecordReader.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/store/text/DrillTextRecordReader.java b/exec/java-exec/src/main/java/org/apache/drill/exec/store/text/DrillTextRecordReader.java
index 0322f36..e25bd74 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/store/text/DrillTextRecordReader.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/store/text/DrillTextRecordReader.java
@@ -231,7 +231,10 @@ public class DrillTextRecordReader extends AbstractRecordReader {
@Override
public void cleanup() {
try {
- reader.close();
+ if (reader != null) {
+ reader.close();
+ reader = null;
+ }
} catch (IOException e) {
logger.warn("Exception closing reader: {}", e);
}
http://git-wip-us.apache.org/repos/asf/drill/blob/826fc5b9/exec/java-exec/src/test/java/org/apache/drill/BaseTestQuery.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/test/java/org/apache/drill/BaseTestQuery.java b/exec/java-exec/src/test/java/org/apache/drill/BaseTestQuery.java
index f8ec090..db1ed34 100644
--- a/exec/java-exec/src/test/java/org/apache/drill/BaseTestQuery.java
+++ b/exec/java-exec/src/test/java/org/apache/drill/BaseTestQuery.java
@@ -59,6 +59,10 @@ import com.google.common.base.Charsets;
import com.google.common.base.Preconditions;
import com.google.common.io.Resources;
+import static org.hamcrest.core.StringContains.containsString;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertThat;
+
public class BaseTestQuery extends ExecTest {
private static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(BaseTestQuery.class);
@@ -319,7 +323,7 @@ public class BaseTestQuery extends ExecTest {
protected static void testNoResult(int interation, String query, Object... args) throws Exception {
query = String.format(query, args);
- logger.debug("Running query:\n--------------\n"+query);
+ logger.debug("Running query:\n--------------\n" + query);
for (int i = 0; i < interation; i++) {
List<QueryDataBatch> results = client.runQuery(QueryType.SQL, query);
for (QueryDataBatch queryDataBatch : results) {
@@ -364,6 +368,24 @@ public class BaseTestQuery extends ExecTest {
test(getFile(file));
}
+ /**
+ * Utility method which tests given query produces a {@link UserException} and the exception message contains
+ * the given message.
+ * @param testSqlQuery Test query
+ * @param expectedErrorMsg Expected error message.
+ */
+ protected static void errorMsgTestHelper(final String testSqlQuery, final String expectedErrorMsg) throws Exception {
+ UserException expException = null;
+ try {
+ test(testSqlQuery);
+ } catch (final UserException ex) {
+ expException = ex;
+ }
+
+ assertNotNull("Expected a UserException", expException);
+ assertThat(expException.getMessage(), containsString(expectedErrorMsg));
+ }
+
public static String getFile(String resource) throws IOException{
URL url = Resources.getResource(resource);
if (url == null) {
http://git-wip-us.apache.org/repos/asf/drill/blob/826fc5b9/exec/java-exec/src/test/java/org/apache/drill/TestDisabledFunctionality.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/test/java/org/apache/drill/TestDisabledFunctionality.java b/exec/java-exec/src/test/java/org/apache/drill/TestDisabledFunctionality.java
index 9420170..adbf653 100644
--- a/exec/java-exec/src/test/java/org/apache/drill/TestDisabledFunctionality.java
+++ b/exec/java-exec/src/test/java/org/apache/drill/TestDisabledFunctionality.java
@@ -18,6 +18,7 @@
package org.apache.drill;
import org.apache.drill.common.exceptions.UserException;
import org.apache.drill.common.util.FileUtils;
+import org.apache.drill.exec.work.ExecErrorConstants;
import org.apache.drill.exec.work.foreman.SqlUnsupportedException;
import org.apache.drill.exec.work.foreman.UnsupportedDataTypeException;
import org.apache.drill.exec.work.foreman.UnsupportedFunctionException;
@@ -355,13 +356,15 @@ public class TestDisabledFunctionality extends BaseTestQuery{
}
}
- @Test(expected = UserException.class) // DRILL-2848
+ @Test // DRILL-2848
public void testDisableDecimalCasts() throws Exception {
- test("select cast('1.2' as decimal(9, 2)) from cp.`employee.json` limit 1");
+ final String query = "select cast('1.2' as decimal(9, 2)) from cp.`employee.json` limit 1";
+ errorMsgTestHelper(query, ExecErrorConstants.DECIMAL_DISABLE_ERR_MSG);
}
- @Test(expected = UserException.class) // DRILL-2848
+ @Test // DRILL-2848
public void testDisableDecimalFromParquet() throws Exception {
- test("select * from cp.`parquet/decimal_dictionary.parquet`");
+ final String query = "select * from cp.`parquet/decimal_dictionary.parquet`";
+ errorMsgTestHelper(query, ExecErrorConstants.DECIMAL_DISABLE_ERR_MSG);
}
}
\ No newline at end of file