Updated Branches: refs/heads/trunk 5e88d43b5 -> 658c15848
SQOOP-976: Incorrect SQL when incremental criteria is text column (Raghav Kumar Gautam via Jarek Jarcec Cecho) Project: http://git-wip-us.apache.org/repos/asf/sqoop/repo Commit: http://git-wip-us.apache.org/repos/asf/sqoop/commit/658c1584 Tree: http://git-wip-us.apache.org/repos/asf/sqoop/tree/658c1584 Diff: http://git-wip-us.apache.org/repos/asf/sqoop/diff/658c1584 Branch: refs/heads/trunk Commit: 658c15848f466e020975d91114120900dd0f9cbf Parents: 5e88d43 Author: Jarek Jarcec Cecho <[email protected]> Authored: Sun Jun 9 08:48:33 2013 -0700 Committer: Jarek Jarcec Cecho <[email protected]> Committed: Sun Jun 9 08:48:33 2013 -0700 ---------------------------------------------------------------------- src/docs/user/import.txt | 5 +- .../org/apache/sqoop/manager/ConnManager.java | 15 +++++ src/java/org/apache/sqoop/tool/ImportTool.java | 6 +- .../cloudera/sqoop/TestIncrementalImport.java | 64 ++++++++++++++++++++ 4 files changed, 88 insertions(+), 2 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/sqoop/blob/658c1584/src/docs/user/import.txt ---------------------------------------------------------------------- diff --git a/src/docs/user/import.txt b/src/docs/user/import.txt index ee10b1c..4a9a316 100644 --- a/src/docs/user/import.txt +++ b/src/docs/user/import.txt @@ -312,7 +312,10 @@ The following arguments control incremental imports: Argument Description --------------------------------------------------------------------- +\--check-column (col)+ Specifies the column to be examined \ - when determining which rows to import. + when determining which rows to import.\ + (the column should not be of type \ + CHAR/NCHAR/VARCHAR/VARNCHAR/\ + LONGVARCHAR/LONGNVARCHAR) +\--incremental (mode)+ Specifies how Sqoop determines which \ rows are new. Legal values for +mode+\ include +append+ and +lastmodified+. http://git-wip-us.apache.org/repos/asf/sqoop/blob/658c1584/src/java/org/apache/sqoop/manager/ConnManager.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/sqoop/manager/ConnManager.java b/src/java/org/apache/sqoop/manager/ConnManager.java index 3549bda..32e736c 100644 --- a/src/java/org/apache/sqoop/manager/ConnManager.java +++ b/src/java/org/apache/sqoop/manager/ConnManager.java @@ -718,5 +718,20 @@ public abstract class ConnManager { public boolean isORMFacilitySelfManaged() { return false; } + + /** + * Determine if a column is char or a char-variant type. + * @return true if column type is CHAR, VARCHAR, LONGVARCHAR + * or their N version. These are used to store strings. + */ + public boolean isCharColumn(int columnType) { + return (columnType == Types.VARCHAR) + || (columnType == Types.NVARCHAR) + || (columnType == Types.CHAR) + || (columnType == Types.NCHAR) + || (columnType == Types.LONGVARCHAR) + || (columnType == Types.LONGNVARCHAR); + } + } http://git-wip-us.apache.org/repos/asf/sqoop/blob/658c1584/src/java/org/apache/sqoop/tool/ImportTool.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/sqoop/tool/ImportTool.java b/src/java/org/apache/sqoop/tool/ImportTool.java index 424d9ec..1c57503 100644 --- a/src/java/org/apache/sqoop/tool/ImportTool.java +++ b/src/java/org/apache/sqoop/tool/ImportTool.java @@ -241,7 +241,7 @@ public class ImportTool extends com.cloudera.sqoop.tool.BaseSqoopTool { || (columnType == Types.TIME); } - /** + /** * Initialize the constraints which set the incremental import range. * @return false if an import is not necessary, because the dataset has not * changed. @@ -270,6 +270,10 @@ public class ImportTool extends com.cloudera.sqoop.tool.BaseSqoopTool { nextIncrementalValue = (nextVal == null) ? null : manager.datetimeToQueryString(nextVal.toString(), checkColumnType); + } else if (manager.isCharColumn(checkColumnType)) { + throw new ImportException("Character column " + + "(" + options.getIncrementalTestColumn() + ") can not be used " + + "to determine which rows to incrementally import."); } else { nextIncrementalValue = (nextVal == null) ? null : nextVal.toString(); } http://git-wip-us.apache.org/repos/asf/sqoop/blob/658c1584/src/test/com/cloudera/sqoop/TestIncrementalImport.java ---------------------------------------------------------------------- diff --git a/src/test/com/cloudera/sqoop/TestIncrementalImport.java b/src/test/com/cloudera/sqoop/TestIncrementalImport.java index cab77ec..02080df 100644 --- a/src/test/com/cloudera/sqoop/TestIncrementalImport.java +++ b/src/test/com/cloudera/sqoop/TestIncrementalImport.java @@ -170,6 +170,30 @@ public class TestIncrementalImport extends TestCase { } /** + * Insert rows with id = [low, hi) into tableName with + * id converted to string. + */ + private void insertIdVarcharRows(String tableName, int low, int hi) + throws SQLException { + LOG.info("Inserting rows in [" + low + ", " + hi + ")"); + SqoopOptions options = new SqoopOptions(); + options.setConnectString(SOURCE_DB_URL); + HsqldbManager manager = new HsqldbManager(options); + Connection c = manager.getConnection(); + PreparedStatement s = null; + try { + s = c.prepareStatement("INSERT INTO " + tableName + " VALUES(?)"); + for (int i = low; i < hi; i++) { + s.setString(1, Integer.toString(i)); + s.executeUpdate(); + } + c.commit(); + } finally { + s.close(); + } + } + + /** * Create a table with an 'id' column full of integers. */ private void createIdTable(String tableName, int insertRows) @@ -212,6 +236,27 @@ public class TestIncrementalImport extends TestCase { } /** + * Create a table with an 'id' column of type varchar(20) + */ + private void createIdVarcharTable(String tableName, + int insertRows) throws SQLException { + SqoopOptions options = new SqoopOptions(); + options.setConnectString(SOURCE_DB_URL); + HsqldbManager manager = new HsqldbManager(options); + Connection c = manager.getConnection(); + PreparedStatement s = null; + try { + s = c.prepareStatement("CREATE TABLE " + tableName + + "(id varchar(20) NOT NULL)"); + s.executeUpdate(); + c.commit(); + insertIdVarcharRows(tableName, 0, insertRows); + } finally { + s.close(); + } + } + + /** * Delete all files in a directory for a table. */ public void clearDir(String tableName) { @@ -754,6 +799,25 @@ public class TestIncrementalImport extends TestCase { assertDirOfNumbers(TABLE_NAME, 20); } + public void testAppendWithString() throws Exception { + // Create a table with string column in it; + // incrementally import it on the string column - it should fail. + + final String TABLE_NAME = "appendString"; + createIdVarcharTable(TABLE_NAME, 10); + + List<String> args = getArgListForTable(TABLE_NAME, false, true); + args.add("--append"); + createJob(TABLE_NAME, args); + try { + runJob(TABLE_NAME); + //the above line should throw an exception otherwise the test has failed + fail("Expected incremental import on varchar column to fail."); + } catch(RuntimeException e) { + //expected + } + } + public void testModifyWithTimestamp() throws Exception { // Create a table with data in it; import it. // Then modify some existing rows, and verify that we only grab
