You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@drill.apache.org by ad...@apache.org on 2015/09/15 14:12:33 UTC

[4/5] drill git commit: DRILL-3347: VARCHAR ResultSet.getObject returned ...hadoop.io.Text, not String.

DRILL-3347: VARCHAR ResultSet.getObject returned ...hadoop.io.Text, not String.

this closes #144
Core fix:
- Fixed {,Nullable}VarCharAccessor's getObject() to return String instead of
  value vector's internal org.apache.hadoop.io.Text.
- Updated unit tests (to expect only String now).
  [DatabaseMetaDataGetColumnsTest, ResultSetMetaDataTest]

Also Added getObject check in tracing proxy test.  [TracingProxyDriverTest]
Changed hard references to Hadoop's Text and JodaTime's Period to strings in
warning check in tracing proxy.  [InvocationReporterImpl]

Cleanup:
- Added @Override annotations.  [SqlAccessors]
- (Unintentionally) fixed (undetected) missing comma.  [ValueVectorTypes.tdd]


Project: http://git-wip-us.apache.org/repos/asf/drill/repo
Commit: http://git-wip-us.apache.org/repos/asf/drill/commit/d6adf5c7
Tree: http://git-wip-us.apache.org/repos/asf/drill/tree/d6adf5c7
Diff: http://git-wip-us.apache.org/repos/asf/drill/diff/d6adf5c7

Branch: refs/heads/master
Commit: d6adf5c7c7d48d4170b2e7dec2ec5e451dda0cea
Parents: dca98ef
Author: dbarclay <db...@maprtech.com>
Authored: Tue Aug 4 16:51:07 2015 -0700
Committer: adeneche <ad...@gmail.com>
Committed: Mon Sep 14 08:00:03 2015 -0700

----------------------------------------------------------------------
 .../src/main/codegen/data/ValueVectorTypes.tdd  |  2 +-
 .../main/codegen/templates/SqlAccessors.java    | 30 ++++++++-
 .../org/apache/drill/jdbc/impl/MetaImpl.java    |  2 +
 .../jdbc/proxy/InvocationReporterImpl.java      |  9 ++-
 .../jdbc/DatabaseMetaDataGetColumnsTest.java    | 64 ++++++++------------
 .../drill/jdbc/ResultSetMetaDataTest.java       |  4 +-
 .../jdbc/proxy/TracingProxyDriverTest.java      |  1 +
 7 files changed, 66 insertions(+), 46 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/drill/blob/d6adf5c7/exec/java-exec/src/main/codegen/data/ValueVectorTypes.tdd
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/codegen/data/ValueVectorTypes.tdd b/exec/java-exec/src/main/codegen/data/ValueVectorTypes.tdd
index 3f69e25..26bf02d 100644
--- a/exec/java-exec/src/main/codegen/data/ValueVectorTypes.tdd
+++ b/exec/java-exec/src/main/codegen/data/ValueVectorTypes.tdd
@@ -151,7 +151,7 @@
       fields: [{name: "start", type: "int"}, {name: "end", type: "int"}, {name: "buffer", type: "DrillBuf"}],
       minor: [
         { class: "VarBinary" , friendlyType: "byte[]" },
-        { class: "VarChar" , friendlyType: "Text" }
+        { class: "VarChar" , friendlyType: "Text" },
         { class: "Var16Char" , friendlyType: "String" }
       ]
     },

http://git-wip-us.apache.org/repos/asf/drill/blob/d6adf5c7/exec/java-exec/src/main/codegen/templates/SqlAccessors.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/codegen/templates/SqlAccessors.java b/exec/java-exec/src/main/codegen/templates/SqlAccessors.java
index 3257499..283c209 100644
--- a/exec/java-exec/src/main/codegen/templates/SqlAccessors.java
+++ b/exec/java-exec/src/main/codegen/templates/SqlAccessors.java
@@ -63,12 +63,17 @@ public class ${name}Accessor extends AbstractSqlAccessor {
    </#if>
   }
 
- <#if minor.class != "TimeStamp" && minor.class != "Time" && minor.class != "Date">
+ <#if minor.class != "VarChar" && minor.class != "TimeStamp"
+   && minor.class != "Time" && minor.class != "Date">
+  <#-- Types whose class for JDBC getObject(...) is same as class from getObject
+       on vector. -->
+
   @Override
   public Class<?> getObjectClass() {
     return ${jdbcObjectClass}.class;
   }
 
+  @Override
   public Object getObject(int index) {
    <#if mode == "Nullable">
     if (ac.isNull(index)) {
@@ -106,6 +111,8 @@ public class ${name}Accessor extends AbstractSqlAccessor {
   <#switch minor.class>
 
     <#case "VarBinary">
+
+    @Override
     public String getString(int index) {
      <#if mode == "Nullable">
       if (ac.isNull(index)) {
@@ -118,6 +125,22 @@ public class ${name}Accessor extends AbstractSqlAccessor {
       <#break>
 
     <#case "VarChar">
+
+    @Override
+    public Class<?> getObjectClass() {
+      return String.class;
+    }
+
+    @Override
+    public String getObject(int index) {
+     <#if mode == "Nullable">
+       if (ac.isNull(index)) {
+         return null;
+       }
+     </#if>
+       return getString(index);
+    }
+
     @Override
     public InputStreamReader getReader(int index) {
      <#if mode == "Nullable">
@@ -140,6 +163,7 @@ public class ${name}Accessor extends AbstractSqlAccessor {
       <#break>
 
     <#case "Var16Char">
+
     @Override
     public InputStreamReader getReader(int index) {
      <#if mode == "Nullable">
@@ -175,6 +199,7 @@ public class ${name}Accessor extends AbstractSqlAccessor {
     return Timestamp.class;
   }
 
+  @Override
   public Object getObject(int index) {
     return getTimestamp(index);
   }
@@ -219,6 +244,7 @@ public class ${name}Accessor extends AbstractSqlAccessor {
     return Date.class;
   }
 
+  @Override
   public Object getObject(int index) {
    <#if mode == "Nullable">
     if (ac.isNull(index)) {
@@ -247,6 +273,7 @@ public class ${name}Accessor extends AbstractSqlAccessor {
     return Timestamp.class;
   }
 
+  @Override
   public Object getObject(int index) {
    <#if mode == "Nullable">
     if (ac.isNull(index)) {
@@ -275,6 +302,7 @@ public class ${name}Accessor extends AbstractSqlAccessor {
     return Time.class;
   }
 
+  @Override
   public Object getObject(int index) {
     return getTime(index);
   }

http://git-wip-us.apache.org/repos/asf/drill/blob/d6adf5c7/exec/jdbc/src/main/java/org/apache/drill/jdbc/impl/MetaImpl.java
----------------------------------------------------------------------
diff --git a/exec/jdbc/src/main/java/org/apache/drill/jdbc/impl/MetaImpl.java b/exec/jdbc/src/main/java/org/apache/drill/jdbc/impl/MetaImpl.java
index f1e3e2c..b1ae12c 100644
--- a/exec/jdbc/src/main/java/org/apache/drill/jdbc/impl/MetaImpl.java
+++ b/exec/jdbc/src/main/java/org/apache/drill/jdbc/impl/MetaImpl.java
@@ -89,6 +89,8 @@ class MetaImpl implements Meta {
       return statement.getResultSet();
 
     } catch (Exception e) {
+      // Wrap in RuntimeException because Avatica's abstract method declarations
+      // didn't allow for SQLException!
       throw new DrillRuntimeException("Failure while attempting to get DatabaseMetadata.", e);
     }
 

http://git-wip-us.apache.org/repos/asf/drill/blob/d6adf5c7/exec/jdbc/src/main/java/org/apache/drill/jdbc/proxy/InvocationReporterImpl.java
----------------------------------------------------------------------
diff --git a/exec/jdbc/src/main/java/org/apache/drill/jdbc/proxy/InvocationReporterImpl.java b/exec/jdbc/src/main/java/org/apache/drill/jdbc/proxy/InvocationReporterImpl.java
index 031a147..0e0d314 100644
--- a/exec/jdbc/src/main/java/org/apache/drill/jdbc/proxy/InvocationReporterImpl.java
+++ b/exec/jdbc/src/main/java/org/apache/drill/jdbc/proxy/InvocationReporterImpl.java
@@ -291,8 +291,13 @@ class InvocationReporterImpl implements InvocationReporter
       else if (
           // Is type to warn about (second case).
           false
-          || rawActualType == org.apache.hadoop.io.Text.class
-          || rawActualType == org.joda.time.Period.class
+          // Note:  Using strings rather than compiled-in class references to
+          // avoid failing when run using JDBC-all Jar, which excludes
+          // org.apache.hadoop.io.Text.
+          // Note:  org.apache.hadoop.io.Text should no longer appear (see
+          // DRILL-3347, but leaving warning in for now in case Text returns).
+          || rawActualType.getName().equals( "org.apache.hadoop.io.Text" )
+          || rawActualType.getName().equals( "org.joda.time.Period" )
           || rawActualType ==
              org.apache.drill.exec.vector.accessor.sql.TimePrintMillis.class
           ) {

http://git-wip-us.apache.org/repos/asf/drill/blob/d6adf5c7/exec/jdbc/src/test/java/org/apache/drill/jdbc/DatabaseMetaDataGetColumnsTest.java
----------------------------------------------------------------------
diff --git a/exec/jdbc/src/test/java/org/apache/drill/jdbc/DatabaseMetaDataGetColumnsTest.java b/exec/jdbc/src/test/java/org/apache/drill/jdbc/DatabaseMetaDataGetColumnsTest.java
index 5676dea..d201140 100644
--- a/exec/jdbc/src/test/java/org/apache/drill/jdbc/DatabaseMetaDataGetColumnsTest.java
+++ b/exec/jdbc/src/test/java/org/apache/drill/jdbc/DatabaseMetaDataGetColumnsTest.java
@@ -412,10 +412,8 @@ public class DatabaseMetaDataGetColumnsTest extends JdbcTestBase {
 
   @Test
   public void test_TABLE_CAT_hasRightClass() throws SQLException {
-    // TODO(DRILL-3347):  Resolve which type(s) to test for:
     assertThat( rowsMetadata.getColumnClassName( 1 ),
-                anyOf( equalTo( String.class.getName() ),
-                       equalTo( org.apache.hadoop.io.Text.class.getName() ) ) );
+                equalTo( String.class.getName() ) );
   }
 
   @Ignore( "until resolved:  any requirement on nullability (DRILL-2420?)" )
@@ -471,10 +469,9 @@ public class DatabaseMetaDataGetColumnsTest extends JdbcTestBase {
 
   @Test
   public void test_TABLE_SCHEM_hasRightClass() throws SQLException {
-    // TODO(DRILL-3347):  Resolve which type(s) to test for:
+
     assertThat( rowsMetadata.getColumnClassName( 2 ),
-                anyOf( equalTo( String.class.getName() ),
-                       equalTo( org.apache.hadoop.io.Text.class.getName() ) ) );
+                equalTo( String.class.getName() ) );
   }
 
   @Ignore( "until resolved:  any requirement on nullability (DRILL-2420?)" )
@@ -521,10 +518,9 @@ public class DatabaseMetaDataGetColumnsTest extends JdbcTestBase {
 
   @Test
   public void test_TABLE_NAME_hasRightClass() throws SQLException {
-    // TODO(DRILL-3347):  Resolve which type(s) to test for:
+
     assertThat( rowsMetadata.getColumnClassName( 3 ),
-                anyOf( equalTo( String.class.getName() ),
-                       equalTo( org.apache.hadoop.io.Text.class.getName() ) ) );
+                equalTo( String.class.getName() ) );
   }
 
   @Ignore( "until resolved:  any requirement on nullability (DRILL-2420?)" )
@@ -579,10 +575,9 @@ public class DatabaseMetaDataGetColumnsTest extends JdbcTestBase {
 
   @Test
   public void test_COLUMN_NAME_hasRightClass() throws SQLException {
-    // TODO(DRILL-3347):  Resolve which type(s) to test for:
+
     assertThat( rowsMetadata.getColumnClassName( 4 ),
-                anyOf( equalTo( String.class.getName() ),
-                       equalTo( org.apache.hadoop.io.Text.class.getName() ) ) );
+                equalTo( String.class.getName() ) );
   }
 
   @Ignore( "until resolved:  any requirement on nullability (DRILL-2420?)" )
@@ -931,10 +926,9 @@ public class DatabaseMetaDataGetColumnsTest extends JdbcTestBase {
 
   @Test
   public void test_TYPE_NAME_hasRightClass() throws SQLException {
-    // TODO(DRILL-3347):  Resolve which type(s) to test for:
+
     assertThat( rowsMetadata.getColumnClassName( 6 ),
-                anyOf( equalTo( String.class.getName() ),
-                       equalTo( org.apache.hadoop.io.Text.class.getName() ) ) );
+                equalTo( String.class.getName() ) );
   }
 
   @Ignore( "until resolved:  any requirement on nullability (DRILL-2420?)" )
@@ -1972,10 +1966,9 @@ public class DatabaseMetaDataGetColumnsTest extends JdbcTestBase {
 
   @Test
   public void test_REMARKS_hasRightClass() throws SQLException {
-    // TODO(DRILL-3347):  Resolve which type(s) to test for:
+
     assertThat( rowsMetadata.getColumnClassName( 12 ),
-                anyOf( equalTo( String.class.getName() ),
-                       equalTo( org.apache.hadoop.io.Text.class.getName() ) ) );
+                equalTo( String.class.getName() ) );
   }
 
   @Test
@@ -2021,10 +2014,9 @@ public class DatabaseMetaDataGetColumnsTest extends JdbcTestBase {
 
   @Test
   public void test_COLUMN_DEF_hasRightClass() throws SQLException {
-    // TODO(DRILL-3347):  Resolve which type(s) to test for:
+
     assertThat( rowsMetadata.getColumnClassName( 13 ),
-                anyOf( equalTo( String.class.getName() ),
-                       equalTo( org.apache.hadoop.io.Text.class.getName() ) ) );
+                equalTo( String.class.getName() ) ); //???Text
   }
 
   @Test
@@ -2540,10 +2532,9 @@ public class DatabaseMetaDataGetColumnsTest extends JdbcTestBase {
 
   @Test
   public void test_IS_NULLABLE_hasRightClass() throws SQLException {
-    // TODO(DRILL-3347):  Resolve which type(s) to test for:
+
     assertThat( rowsMetadata.getColumnClassName( 18 ),
-                anyOf( equalTo( String.class.getName() ),
-                       equalTo( org.apache.hadoop.io.Text.class.getName() ) ) );
+                equalTo( String.class.getName() ) );
   }
 
   @Ignore( "until resolved:  any requirement on nullability (DRILL-2420?)" )
@@ -2590,10 +2581,9 @@ public class DatabaseMetaDataGetColumnsTest extends JdbcTestBase {
 
   @Test
   public void test_SCOPE_CATALOG_hasRightClass() throws SQLException {
-    // TODO(DRILL-3347):  Resolve which type(s) to test for:
+
     assertThat( rowsMetadata.getColumnClassName( 19 ),
-                anyOf( equalTo( String.class.getName() ),
-                       equalTo( org.apache.hadoop.io.Text.class.getName() ) ) );
+                equalTo( String.class.getName() ) );
   }
 
   @Test
@@ -2639,10 +2629,9 @@ public class DatabaseMetaDataGetColumnsTest extends JdbcTestBase {
 
   @Test
   public void test_SCOPE_SCHEMA_hasRightClass() throws SQLException {
-    // TODO(DRILL-3347):  Resolve which type(s) to test for:
+
     assertThat( rowsMetadata.getColumnClassName( 20 ),
-                anyOf( equalTo( String.class.getName() ),
-                       equalTo( org.apache.hadoop.io.Text.class.getName() ) ) );
+                equalTo( String.class.getName() ) );
   }
 
   @Test
@@ -2688,10 +2677,9 @@ public class DatabaseMetaDataGetColumnsTest extends JdbcTestBase {
 
   @Test
   public void test_SCOPE_TABLE_hasRightClass() throws SQLException {
-    // TODO(DRILL-3347):  Resolve which type(s) to test for:
+
     assertThat( rowsMetadata.getColumnClassName( 21 ),
-                anyOf( equalTo( String.class.getName() ),
-                       equalTo( org.apache.hadoop.io.Text.class.getName() ) ) );
+                equalTo( String.class.getName() ) );
   }
 
   @Test
@@ -2791,10 +2779,9 @@ public class DatabaseMetaDataGetColumnsTest extends JdbcTestBase {
 
   @Test
   public void test_IS_AUTOINCREMENT_hasRightClass() throws SQLException {
-    // TODO(DRILL-3347):  Resolve which type(s) to test for:
+
     assertThat( rowsMetadata.getColumnClassName( 23 ),
-                anyOf( equalTo( String.class.getName() ),
-                       equalTo( org.apache.hadoop.io.Text.class.getName() ) ) );
+                equalTo( String.class.getName() ) );
   }
 
   @Test
@@ -2844,10 +2831,9 @@ public class DatabaseMetaDataGetColumnsTest extends JdbcTestBase {
 
   @Test
   public void test_IS_GENERATEDCOLUMN_hasRightClass() throws SQLException {
-    // TODO(DRILL-3347):  Resolve which type(s) to test for:
+
     assertThat( rowsMetadata.getColumnClassName( 24 ),
-                anyOf( equalTo( String.class.getName() ),
-                       equalTo( org.apache.hadoop.io.Text.class.getName() ) ) );
+                equalTo( String.class.getName() ) );
   }
 
   @Test

http://git-wip-us.apache.org/repos/asf/drill/blob/d6adf5c7/exec/jdbc/src/test/java/org/apache/drill/jdbc/ResultSetMetaDataTest.java
----------------------------------------------------------------------
diff --git a/exec/jdbc/src/test/java/org/apache/drill/jdbc/ResultSetMetaDataTest.java b/exec/jdbc/src/test/java/org/apache/drill/jdbc/ResultSetMetaDataTest.java
index ba5435f..d8800fb 100644
--- a/exec/jdbc/src/test/java/org/apache/drill/jdbc/ResultSetMetaDataTest.java
+++ b/exec/jdbc/src/test/java/org/apache/drill/jdbc/ResultSetMetaDataTest.java
@@ -988,10 +988,8 @@ public class ResultSetMetaDataTest extends JdbcTestBase {
 
   @Test
   public void test_getColumnClassName_forVARCHAR_10_isString() throws SQLException {
-    // TODO(DRILL-3347):  Resolve which type(s) to test for:
     assertThat( rowMetadata.getColumnClassName( ordReqVARCHAR_10 ),
-                anyOf( equalTo( String.class.getName() ),
-                       equalTo( org.apache.hadoop.io.Text.class.getName() ) ) );
+                equalTo( String.class.getName() ) );
   }
 
   @Test

http://git-wip-us.apache.org/repos/asf/drill/blob/d6adf5c7/exec/jdbc/src/test/java/org/apache/drill/jdbc/proxy/TracingProxyDriverTest.java
----------------------------------------------------------------------
diff --git a/exec/jdbc/src/test/java/org/apache/drill/jdbc/proxy/TracingProxyDriverTest.java b/exec/jdbc/src/test/java/org/apache/drill/jdbc/proxy/TracingProxyDriverTest.java
index 6e8a17c..55431bf 100644
--- a/exec/jdbc/src/test/java/org/apache/drill/jdbc/proxy/TracingProxyDriverTest.java
+++ b/exec/jdbc/src/test/java/org/apache/drill/jdbc/proxy/TracingProxyDriverTest.java
@@ -67,6 +67,7 @@ public class TracingProxyDriverTest extends DrillTest {
           stmt.executeQuery( "SELECT * FROM INFORMATION_SCHEMA.CATALOGS" );
       assertTrue( rs.next() );
       assertThat( rs.getString( 1 ), equalTo( "DRILL" ) );
+      assertThat( rs.getObject( 1 ), equalTo( (Object) "DRILL" ) );
     }
   }