----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31160/#review78788 -----------------------------------------------------------
Ship it! Updated patch looks ok to me. Couple of suggestions about testing: the testing section says 'added unit tests for HJ and MJ' but I did not see those...I am thinking it is already covered by existing HJ and MJ tests as long as you set slice_target = 1 to force exchanges. Note that TpchDistributed sets slice_target = 1 but most of those plans are HashJoin plans. So you might want to check that out. Also, it would be useful to have a negative test case where we don't expect the hash as double (e.g for aggregates or for non-numeric types). - Aman Sinha On April 3, 2015, 8:13 a.m., Mehant Baid wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/31160/ > ----------------------------------------------------------- > > (Updated April 3, 2015, 8:13 a.m.) > > > Review request for drill and Aman Sinha. > > > Repository: drill-git > > > Description > ------- > > If the return type of the two expressions in the join condition are of > different types then we need to inject a cast on one of the sides for > comparison functions to work as expected > > > Diffs > ----- > > > exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/Hash64AsDouble.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/Hash64Functions.java > 57154ed > > exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/Hash64FunctionsWithSeed.java > b9ec956 > > exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/Hash64WithSeedAsDouble.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/ChainedHashTable.java > 84a2956 > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/PrelUtil.java > f7c144f > > exec/java-exec/src/main/java/org/apache/drill/exec/resolver/TypeCastRules.java > d8652f2 > exec/java-exec/src/test/java/org/apache/drill/TestFunctionsQuery.java > f1005ab > > Diff: https://reviews.apache.org/r/31160/diff/ > > > Testing > ------- > > Added unit tests with hash join and merge join > > > Thanks, > > Mehant Baid > >
