Return-Path: X-Original-To: apmail-drill-commits-archive@www.apache.org Delivered-To: apmail-drill-commits-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 7283917F20 for ; Tue, 15 Sep 2015 12:12:31 +0000 (UTC) Received: (qmail 19299 invoked by uid 500); 15 Sep 2015 12:12:31 -0000 Delivered-To: apmail-drill-commits-archive@drill.apache.org Received: (qmail 19250 invoked by uid 500); 15 Sep 2015 12:12:31 -0000 Mailing-List: contact commits-help@drill.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: commits@drill.apache.org Delivered-To: mailing list commits@drill.apache.org Received: (qmail 19218 invoked by uid 99); 15 Sep 2015 12:12:31 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 15 Sep 2015 12:12:31 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id 08CA5E009E; Tue, 15 Sep 2015 12:12:31 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: adeneche@apache.org To: commits@drill.apache.org Date: Tue, 15 Sep 2015 12:12:33 -0000 Message-Id: <752054dcb9b347078b98d5f51cee8ba0@git.apache.org> In-Reply-To: <978fc468aebb46e7a90dc9b1ddc14225@git.apache.org> References: <978fc468aebb46e7a90dc9b1ddc14225@git.apache.org> X-Mailer: ASF-Git Admin Mailer Subject: [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 Authored: Tue Aug 4 16:51:07 2015 -0700 Committer: adeneche 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 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; + } + + 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" ) ); } }