drill-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ve...@apache.org
Subject drill git commit: DRILL-3017: Safeguard against NPEs in RecordReader.cleanup()s
Date Mon, 11 May 2015 02:07:55 GMT
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 <venki.korukanti@gmail.com>
Authored: Sun May 10 15:31:58 2015 -0700
Committer: vkorukanti <venki.korukanti@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


Mime
View raw message