----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57499/#review173716 -----------------------------------------------------------
Hi, Thank you for this patch, this seems like a very useful change! There are a few suggestions/questions I've added, please look through them and fix/comment as needed :). Also thank you for creating a test case for this, that was a lot of help with the review and testing! Thanks, Anna src/java/org/apache/sqoop/hbase/ToStringPutTransformer.java Line 191 (original), 196 (patched) <https://reviews.apache.org/r/57499/#comment250697> it seems Put class' add method was deprecated in hbase 1.0.0 and we're currently using hbase 1.2.4, please use put.addColumn instead. src/java/org/apache/sqoop/hbase/ToStringPutTransformer.java Lines 205 (patched) <https://reviews.apache.org/r/57499/#comment250696> it seems deleteColumn was deprecated in hbase 1.0.0 and we're currently using hbase 1.2.4, please use delete.addColumn instead. src/java/org/apache/sqoop/hbase/ToStringPutTransformer.java Lines 211 (patched) <https://reviews.apache.org/r/57499/#comment246762> It might be cleaner if you initialize the list of mutations sooner and add the correct put or delete to the list based on the val == null instead of carrying the booleans isPut and isDelete (or create a mutation object and carry that around, in that case you can create an immutable list instead of a regular ArrayList). src/test/com/cloudera/sqoop/hbase/HBaseImportTest.java Lines 85 (patched) <https://reviews.apache.org/r/57499/#comment250725> Currently we're only testing the null update for lastmodified. Based on my understanding this should not affect append mode, could you please add a test case to verify that this change does not affect append mode, if you agree with my assumption. src/test/com/cloudera/sqoop/hbase/HBaseTestCase.java Line 118 (original), 125-156 (patched) <https://reviews.apache.org/r/57499/#comment250724> Quite a bit of this function is already in the original getArgv function in the HBaseTestCase class, please reuse/extract-refactor that part of the method. - Anna Szonyi On April 9, 2017, 5:49 p.m., Jilani Shaik wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/57499/ > ----------------------------------------------------------- > > (Updated April 9, 2017, 5:49 p.m.) > > > Review request for Sqoop, Boglarka Egyed, Attila Szabo, and Anna Szonyi. > > > Bugs: SQOOP-3149 > https://issues.apache.org/jira/browse/SQOOP-3149 > > > Repository: sqoop-trunk > > > Description > ------- > > HBase delete is added as part of incremental data import, Modified the return > type of method which is responsible for holding list of Put objects to List > of Mutation objects and at the time of execution of insert or delete using > the type of object in Mutation, executed the actaul operation either insert > or delete. > > Jira ticket for the same: https://issues.apache.org/jira/browse/SQOOP-3149 > > Similar ticket to above: SQOOP-3125 > > > Diffs > ----- > > src/java/org/apache/sqoop/hbase/HBasePutProcessor.java fdbe1276 > src/java/org/apache/sqoop/hbase/PutTransformer.java 533467e5 > src/java/org/apache/sqoop/hbase/ToStringPutTransformer.java 363e1456 > src/java/org/apache/sqoop/mapreduce/HBaseBulkImportMapper.java 58ccee7b > src/test/com/cloudera/sqoop/hbase/HBaseImportTest.java fa14a013 > src/test/com/cloudera/sqoop/hbase/HBaseTestCase.java a054eb66 > src/test/com/cloudera/sqoop/testutil/BaseSqoopTestCase.java 6310a398 > > > Diff: https://reviews.apache.org/r/57499/diff/4/ > > > Testing > ------- > > Executed with jar prepared with these changes into hadoop cluster and tested > bot import and then incremental import. > > > File Attachments > ---------------- > > HBase delete support for incremental import > > https://reviews.apache.org/media/uploaded/files/2017/03/11/5b1895fd-4c6b-42fa-8a92-4e093153e370__hbase_delete_support_in_incremental_import > updated with unit test and based on below suggestions > > https://reviews.apache.org/media/uploaded/files/2017/03/22/708f63ba-2d8a-4a47-ab67-c1d2776354fd__hbase_delete_support_in_incremental_unit_test_also > > > Thanks, > > Jilani Shaik > >
