----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26694/#review57273 -----------------------------------------------------------
This looks pretty good to me overall, thanks for doing this! I just have some minor comments. datafu-pig/src/main/java/datafu/pig/bags/BagJoin.java <https://reviews.apache.org/r/26694/#comment97829> Doc not accurate datafu-pig/src/main/java/datafu/pig/bags/BagJoin.java <https://reviews.apache.org/r/26694/#comment97822> I think "full" would be better than "outer". Thoughts? datafu-pig/src/main/java/datafu/pig/bags/BagJoin.java <https://reviews.apache.org/r/26694/#comment97824> An enum with values LEFT and FULL would be clearer than two flags probably. datafu-pig/src/main/java/datafu/pig/bags/BagJoin.java <https://reviews.apache.org/r/26694/#comment97828> comment not accurate - Matthew Hayes On Oct. 14, 2014, 5:49 p.m., Jason Reid wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/26694/ > ----------------------------------------------------------- > > (Updated Oct. 14, 2014, 5:49 p.m.) > > > Review request for DataFu. > > > Repository: datafu > > > Description > ------- > > DATAFU-70: Add BagFullOuterJoin > > > Diffs > ----- > > datafu-pig/src/main/java/datafu/pig/bags/BagJoin.java PRE-CREATION > datafu-pig/src/main/java/datafu/pig/bags/BagLeftOuterJoin.java ba6bc11 > datafu-pig/src/test/java/datafu/test/pig/bags/BagTests.java 6d21dbe > > Diff: https://reviews.apache.org/r/26694/diff/ > > > Testing > ------- > > Added bagJoinFullOuterTest to BagTests which uses the same input as the > bagLeftOuterJoinTest but asserts the full outer join logic in the output. I > have also refactored the existing BagLeftOuterJoin class to use the generic > BagJoin class and the existing test continues to pass without modification. > > > Thanks, > > Jason Reid > >
