> On 2012-03-14 03:05:47, Francis Liu wrote: > > http://svn.apache.org/repos/asf/incubator/hcatalog/trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/HBaseHCatStorageHandler.java, > > line 163 > > <https://reviews.apache.org/r/4322/diff/2/?file=91963#file91963line163> > > > > Does this work without the foolproof code? Let's try to contain the bug > > with pig in HCatStorer. It'll just add more complexity. > > Rohini Palaniswamy wrote: > Nothing complex. Just added to check to ensure we don't create > transaction again if same jobConf is passed again with a different > OutputJobInfo. In Pig's case we fixed to pass same OutputJobInfo but it is > with a different JobConf . Added a test case assert for the two cases.
You are adding logic to support misuse. Granted we have to support it because of a bug in Pig. Let's try and localize it in HCatStorer. Adding logic here even little would make it confusing as a developer would have to understand how pig works. I'm fine with this going in for now but it should be removed once HCATALOG-314 is out. - Francis ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4322/#review5917 ----------------------------------------------------------- On 2012-03-14 06:48:57, Rohini Palaniswamy wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/4322/ > ----------------------------------------------------------- > > (Updated 2012-03-14 06:48:57) > > > Review request for hcatalog, Alan Gates, Francis Liu, and Vandana > Ayyalasomayajula. > > > Summary > ------- > > Fixes to get Hbase storagehandler working with Pig. > > > 1)Includes changes to HCatStorer used by Pig. Modified HCatStorer to pass the > same OutputJobInfo in the multiple calls made in Pig to > HCatOutputFormat.setOutput. Else we ended up creating multiple orphaned > transactions which were never committed in zookeeper. > 2) Moved stuff from getInputSplits and checkoutputSpecs to configure methods > in storagehandler. > 3) Fixed a logic error in hbase record reader. > > > This addresses bug HCATALOG-302. > https://issues.apache.org/jira/browse/HCATALOG-302 > > > Diffs > ----- > > > http://svn.apache.org/repos/asf/incubator/hcatalog/trunk/src/java/org/apache/hcatalog/pig/HCatStorer.java > 1299921 > > http://svn.apache.org/repos/asf/incubator/hcatalog/trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/HBaseBulkOutputFormat.java > 1299921 > > http://svn.apache.org/repos/asf/incubator/hcatalog/trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/HBaseDirectOutputFormat.java > 1299921 > > http://svn.apache.org/repos/asf/incubator/hcatalog/trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/HBaseHCatStorageHandler.java > 1299921 > > http://svn.apache.org/repos/asf/incubator/hcatalog/trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/HBaseInputFormat.java > 1299921 > > http://svn.apache.org/repos/asf/incubator/hcatalog/trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/HbaseSnapshotRecordReader.java > 1299921 > > http://svn.apache.org/repos/asf/incubator/hcatalog/trunk/storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/TestHBaseBulkOutputFormat.java > 1299921 > > http://svn.apache.org/repos/asf/incubator/hcatalog/trunk/storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/TestHBaseDirectOutputFormat.java > 1299921 > > http://svn.apache.org/repos/asf/incubator/hcatalog/trunk/storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/TestHBaseInputFormat.java > 1299921 > > Diff: https://reviews.apache.org/r/4322/diff > > > Testing > ------- > > Unit tests and Integration testing done. > > > Thanks, > > Rohini > >
