----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28782/#review64186 -----------------------------------------------------------
Ship it! LGTM. IMO the most important part of the story is being able to add to this over time. For instance, maybe we can support multiple formats of dates, times, and timestamps in text eventually. Also, we should be able to add a transformation layer that can remove timezone information if needed if we so choose to make all timestamps that don't include timezone information simply have UTC offset. connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/util/SqlTypesUtils.java <https://reviews.apache.org/r/28782/#comment106655> TIMESTAMP should have timezone information? core/src/main/java/org/apache/sqoop/driver/JobManager.java <https://reviews.apache.org/r/28782/#comment106654> Probably not necessary. The Joda jar will be included when calling jobRequest.addJarForClass(LocalDate.class) - Abraham Elmahrek On Dec. 6, 2014, 3:35 a.m., Veena Basavaraj wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/28782/ > ----------------------------------------------------------- > > (Updated Dec. 6, 2014, 3:35 a.m.) > > > Review request for Sqoop. > > > Bugs: SQOO-1845 > https://issues.apache.org/jira/browse/SQOO-1845 > > > Repository: sqoop-sqoop2 > > > Description > ------- > > see jira > > > Diffs > ----- > > common/src/main/java/org/apache/sqoop/json/util/SchemaSerialization.java > 2f5fadc > common/src/main/java/org/apache/sqoop/schema/type/DateTime.java 791542a > common/src/main/java/org/apache/sqoop/schema/type/Time.java 9765eab > > common/src/test/java/org/apache/sqoop/json/util/TestSchemaSerialization.java > 3d31600 > > connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/util/SqlTypesUtils.java > 9d50dc3 > > connector/connector-sdk/src/main/java/org/apache/sqoop/connector/idf/CSVIntermediateDataFormat.java > daa51eb > > connector/connector-sdk/src/test/java/org/apache/sqoop/connector/idf/TestCSVIntermediateDataFormat.java > e66c897 > core/src/main/java/org/apache/sqoop/driver/JobManager.java f4f5561 > > Diff: https://reviews.apache.org/r/28782/diff/ > > > Testing > ------- > > yes > > > Thanks, > > Veena Basavaraj > >
