----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5248/#review8133 -----------------------------------------------------------
Ship it! Hi Cheolsoo, overall changes are great, I do have just two minor comments: /src/java/org/apache/sqoop/hive/HiveImport.java <https://reviews.apache.org/r/5248/#comment17655> Could you also fix javadoc comment for changed parameter? /src/java/org/apache/sqoop/hive/HiveImport.java <https://reviews.apache.org/r/5248/#comment17654> FileSystem.delete(Path) is deprecated method, could you change it to delete(Path, boolean)? - Jarek On 2012-05-25 23:20:24, Cheolsoo Park wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/5248/ > ----------------------------------------------------------- > > (Updated 2012-05-25 23:20:24) > > > Review request for Sqoop and Jarek Cecho. > > > Summary > ------- > > Currently, the table name (set by --table) has to be the same as the target > dir (set by --target-dir) for hive import. But it would be nice if the target > dir could be set to a different name than the table name. > > The changes include: > > 1) Implement the following logic in TableDefWriter: > - If target dir is specified, use it as the source. > - Otherwise, use the name of input table as the source. > > The original logic was less flexible: if input table is specified, use it as > the source; otherwise, use target dir as the source. However, this disallows > target dir to be different from input table name when importing the entire > table. With the new logic, the user can freely specify input table, target > dir, and output table in any case. > > 2) Clean up output-path-related code in Hive import. > > 3) Add a unit test that verifies that TableDefWriter generates correct > statements given input table, output table, and target-dir are all different. > > > This addresses bug SQOOP-483. > https://issues.apache.org/jira/browse/SQOOP-483 > > > Diffs > ----- > > /src/test/com/cloudera/sqoop/hive/TestTableDefWriter.java 1342749 > /src/java/org/apache/sqoop/hive/TableDefWriter.java 1342749 > /src/java/org/apache/sqoop/hive/HiveImport.java 1342749 > > Diff: https://reviews.apache.org/r/5248/diff > > > Testing > ------- > > 1) Ran ant test, ant test -Dthirdparty=true, and ant checkstyle. > > 2) Tested with Hive the following scenarios: > - sqoop hive-import ... --table FOO > - sqoop hive-import ... --table FOO --hive table FOO > - sqoop hive-import ... --table FOO --hive table BAR > - sqoop hive-import ... --table FOO --hive-table FOO --target-dir FOO > - sqoop hive-import ... --table FOO --hive-table FOO --target-dir BAR > - sqoop hive-import ... --table FOO --hive-table BAR --target-dir FOO > - sqoop hive-import ... --table FOO --hive-table BAR --target-dir BAR > - sqoop hive-import ... --table FOO --hive-table BAR --target-dir BAZ > > > Thanks, > > Cheolsoo > >
