[ 
https://issues.apache.org/jira/browse/PIG-1205?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12841937#action_12841937
 ] 

Pradeep Kamath commented on PIG-1205:
-------------------------------------

Review comments:
1) The top level comment in HBaseStorage reads - "A Hbase loader" - am 
wondering if it is worth keeping it a loader (maybe change the name to 
HBaseLoader) and create a separate Storer which extends StoreFunc rather than 
have HBaseStorage implement StoreFuncInterface - by extending the StoreFunc, if 
new functions with default implementations are added then the Storer will not 
need to change. The disadvantage is if we call the loader HBaseLoader, existing 
users of HBaseStorage would have to change their scripts to use HBaseLoader 
instead. This is just a suggestion - I am fine if HBaseStorage does both load 
and store and implements StoreFuncInterface - Jeff I will let you decide which 
is better. If you choose to do both load and store in HBaseStorage change the 
top level comment accordingly.
2) The following method implementation should change from:

{code}
      @Override                                                                 
                                                                                
                                                                           
      public String relToAbsPathForStoreLocation(String location, Path curDir)  
                                                                                
                                                                           
              throws IOException {                                              
                                                                                
                                                                           
          // TODO Auto-generated method stub                                    
                                                                                
                                                                           
          return null;                                                          
                                                                                
                                                                           
      }               
{code}

to

{code}
      @Override                                                                 
                                                                                
                                                                           
      public String relToAbsPathForStoreLocation(String location, Path curDir)  
                                                                                
                                                                           
              throws IOException {                                              
                                                                                
                                                                           
          return location;                                                      
                                                                                
                                                                               
      }               
{code}

Also, do address the javadoc/javac issues reported above.

If the above are addressed, +1 for the patch (I don't have enough HBase 
knowledge to review the HBase specific code - I have only reviewed the use of 
load/store API).


> Enhance HBaseStorage-- Make it support loading row key and implement StoreFunc
> ------------------------------------------------------------------------------
>
>                 Key: PIG-1205
>                 URL: https://issues.apache.org/jira/browse/PIG-1205
>             Project: Pig
>          Issue Type: Sub-task
>    Affects Versions: 0.7.0
>            Reporter: Jeff Zhang
>            Assignee: Jeff Zhang
>             Fix For: 0.7.0
>
>         Attachments: PIG_1205.patch, PIG_1205_2.patch
>
>


-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply via email to