----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32415/#review77562 -----------------------------------------------------------
Thank you for the patch Veena. I have couple of comments: connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcToInitializer.java <https://reviews.apache.org/r/32415/#comment125703> I think that we should expose ability for user to specify an updateColumn in case that the table doesn't have a primary key. I know that it sounds unlikely, but I've seen a lot of users that actually had such environments and need to export data into them. Also I think that we should add support for composite primary keys / update columns (e.g. if we need to identify the update row based on multiple columns). connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/configuration/IncrementalWriteConfig.java <https://reviews.apache.org/r/32415/#comment125683> Missing header file. connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/configuration/IncrementalWriteConfig.java <https://reviews.apache.org/r/32415/#comment125682> I believe that USER_ONLY is the default value right? So we perhaps don't need to specify it explicitly? connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/configuration/IncrementalWriteMode.java <https://reviews.apache.org/r/32415/#comment125684> Missing header file connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/configuration/IncrementalWriteMode.java <https://reviews.apache.org/r/32415/#comment125686> It seems that the UPSERT is not implemented yet, so I would suggest to not expose such configuration option to the user and "comment it out" similarly as we have for the "DELETE" example. (unless you will add it in subsequent patch as mentioned on the JIRA). Jarcec - Jarek Cecho On March 23, 2015, 9:15 p.m., Veena Basavaraj wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/32415/ > ----------------------------------------------------------- > > (Updated March 23, 2015, 9:15 p.m.) > > > Review request for Sqoop and Jarek Cecho. > > > Bugs: SQOOP-1810 > https://issues.apache.org/jira/browse/SQOOP-1810 > > > Repository: sqoop-sqoop2 > > > Description > ------- > > see JIRA for details on tests > > > Diffs > ----- > > > common/src/main/java/org/apache/sqoop/error/code/GenericJdbcConnectorError.java > f18acbd > > connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcExecutor.java > 5af34a5 > > connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcLoader.java > ab1ac86 > > connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcToInitializer.java > 400c0f2 > > connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/configuration/IncrementalWriteConfig.java > PRE-CREATION > > connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/configuration/IncrementalWriteMode.java > PRE-CREATION > > connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/configuration/ToJobConfiguration.java > fd5d54b > > connector/connector-generic-jdbc/src/main/resources/generic-jdbc-connector-config.properties > 52bf631 > > Diff: https://reviews.apache.org/r/32415/diff/ > > > Testing > ------- > > see JIRA for details on tests > > > Thanks, > > Veena Basavaraj > >
