----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4623/#review6687 -----------------------------------------------------------
/trunk/src/java/org/apache/hcatalog/pig/HCatLoader.java <https://reviews.apache.org/r/4623/#comment14492> you prolly shouldn't repopulate the context on backend. So the first part of the condition is not needed. /trunk/src/java/org/apache/hcatalog/pig/HCatLoader.java <https://reviews.apache.org/r/4623/#comment14495> add comment about storing the delta. /trunk/src/java/org/apache/hcatalog/pig/HCatLoader.java <https://reviews.apache.org/r/4623/#comment14494> This could be replaced with: for(Map.Entry<String,String> keyValue : job.getConfiguration()) /trunk/src/java/org/apache/hcatalog/pig/HCatStorer.java <https://reviews.apache.org/r/4623/#comment14498> don't need to run this repopulate on the back end. the else statement shouldn't run on the backend either. propbably throw an exception if LOCATION_SET is not set and you're in the backend. /trunk/src/java/org/apache/hcatalog/pig/HCatStorer.java <https://reviews.apache.org/r/4623/#comment14497> credentials shouldn't be stored in udfProps....as this gets passed to the backend most likely in plaintext via jobconf which is unsecure. It should stay on the frontend and just repopulate the job object. Take a look at Security.java on how to do this. /trunk/src/java/org/apache/hcatalog/pig/HCatStorer.java <https://reviews.apache.org/r/4623/#comment14499> I don't think you're gonna need this if statement anymore? - Francis On 2012-04-03 21:31:04, Vandana Ayyalasomayajula wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/4623/ > ----------------------------------------------------------- > > (Updated 2012-04-03 21:31:04) > > > Review request for hcatalog, Francis Liu and Rohini Palaniswamy. > > > Summary > ------- > > HCatStorer.setStoreLocation() is called more than once by pig. Which ends up > calling HCatOutputFormat.setOutput multiple times. HCatOutputFormat.setOutput > should be called only once, the values that it changed must be tracked and > reapplied to job.getconfiguration() the succeeding times it is called. > HCatLoader may have the same issue and should be investigated as well. > > > This addresses bug hcatalog-314. > https://issues.apache.org/jira/browse/hcatalog-314 > > > Diffs > ----- > > /trunk/src/java/org/apache/hcatalog/common/HCatConstants.java 1309001 > /trunk/src/java/org/apache/hcatalog/pig/HCatLoader.java 1309001 > /trunk/src/java/org/apache/hcatalog/pig/HCatStorer.java 1309001 > > Diff: https://reviews.apache.org/r/4623/diff > > > Testing > ------- > > The trunk builds are currently failing. I will run the entire test suite once > the build is fine. > > > Thanks, > > Vandana > >
