You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by Mehant Baid <ba...@gmail.com> on 2015/04/02 20:57:14 UTC
Re: Review Request 31160: DRILL-2244: Implicit cast for join
conditions
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31160/
-----------------------------------------------------------
(Updated April 2, 2015, 6:57 p.m.)
Review request for drill and Aman Sinha.
Changes
-------
Now we hash all numeric data types as double, this would fix the distribution issue and we can apply implicit casting in the join.
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 (updated)
-----
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/physical/impl/common/ChainedHashTable.java 84a2956
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
Re: Review Request 31160: DRILL-2244: Implicit cast for join
conditions
Posted by Aman Sinha <as...@maprtech.com>.
-----------------------------------------------------------
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
>
>
Re: Review Request 31160: DRILL-2244: Implicit cast for join
conditions
Posted by Mehant Baid <ba...@gmail.com>.
-----------------------------------------------------------
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.
Changes
-------
Updated patch to address review comment from Aman to not use hashing as double for aggregates and only to use it for joins and distribution.
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 (updated)
-----
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
Re: Review Request 31160: DRILL-2244: Implicit cast for join
conditions
Posted by Mehant Baid <ba...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31160/
-----------------------------------------------------------
(Updated April 3, 2015, 12:48 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 (updated)
-----
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/physical/impl/common/ChainedHashTable.java 84a2956
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