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 2014/04/26 09:40:44 UTC
Review Request 20742: Allow implicit cast to be applied in both directions
for certain data types
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20742/
-----------------------------------------------------------
Review request for drill, Jacques Nadeau and Jinfeng Ni.
Bugs: DRILL-577
https://issues.apache.org/jira/browse/DRILL-577
Repository: drill-git
Description
-------
Currently implicit cast works as per the precedence rules. However based on some functions it might be useful allow the implicit cast to work in the opposite direction as specified by the precedence map.
For example: As per the precedence rules, we can implicitly cast from VARCHAR ---> BIGINT. However for some functions (eg: substr, concat) it might be useful to implicitly cast from BIGINT ---> VARCHAR.
ResolverTypePrecedence.java:
Added a new set of secondary rules that allow for implicit cast to work in the other direction. Currently these secondary set of rules only allow to cast to VARCHAR, so we don't have cost, if we add more rules here we can add cost to determine which cast function to pick.
ExpressionTreeMaterializer.java:
Fixed bug in visitFunctionCall() that does not add an argument(length) while casting to variable width types
TypeCastRules.java
Added logic to see if we can apply implicit cast based on the secondary rules while matching a given call to a DrillFuncHolder.
Diffs
-----
exec/java-exec/src/main/java/org/apache/drill/exec/expr/ExpressionTreeMaterializer.java a602d82
exec/java-exec/src/main/java/org/apache/drill/exec/resolver/ResolverTypePrecedence.java f6d83e2
exec/java-exec/src/main/java/org/apache/drill/exec/resolver/TypeCastRules.java 3aab08f
exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestReverseImplicitCast.java PRE-CREATION
exec/java-exec/src/test/resources/functions/cast/two_way_implicit_cast.json PRE-CREATION
Diff: https://reviews.apache.org/r/20742/diff/
Testing
-------
Added test to check that implicit cast works both ways.
8 + '2'
substr(10123, 1, 3)
Thanks,
Mehant Baid
Re: Review Request 20742: Allow implicit cast to be applied in both
directions for certain data types
Posted by Jinfeng Ni <jn...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20742/#review42336
-----------------------------------------------------------
Ship it!
Ship It!
- Jinfeng Ni
On April 26, 2014, 12:40 a.m., Mehant Baid wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20742/
> -----------------------------------------------------------
>
> (Updated April 26, 2014, 12:40 a.m.)
>
>
> Review request for drill, Jacques Nadeau and Jinfeng Ni.
>
>
> Bugs: DRILL-577
> https://issues.apache.org/jira/browse/DRILL-577
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> Currently implicit cast works as per the precedence rules. However based on some functions it might be useful allow the implicit cast to work in the opposite direction as specified by the precedence map.
> For example: As per the precedence rules, we can implicitly cast from VARCHAR ---> BIGINT. However for some functions (eg: substr, concat) it might be useful to implicitly cast from BIGINT ---> VARCHAR.
>
> ResolverTypePrecedence.java:
> Added a new set of secondary rules that allow for implicit cast to work in the other direction. Currently these secondary set of rules only allow to cast to VARCHAR, so we don't have cost, if we add more rules here we can add cost to determine which cast function to pick.
>
> ExpressionTreeMaterializer.java:
> Fixed bug in visitFunctionCall() that does not add an argument(length) while casting to variable width types
>
> TypeCastRules.java
> Added logic to see if we can apply implicit cast based on the secondary rules while matching a given call to a DrillFuncHolder.
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/java/org/apache/drill/exec/expr/ExpressionTreeMaterializer.java a602d82
> exec/java-exec/src/main/java/org/apache/drill/exec/resolver/ResolverTypePrecedence.java f6d83e2
> exec/java-exec/src/main/java/org/apache/drill/exec/resolver/TypeCastRules.java 3aab08f
> exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestReverseImplicitCast.java PRE-CREATION
> exec/java-exec/src/test/resources/functions/cast/two_way_implicit_cast.json PRE-CREATION
>
> Diff: https://reviews.apache.org/r/20742/diff/
>
>
> Testing
> -------
>
> Added test to check that implicit cast works both ways.
>
> 8 + '2'
> substr(10123, 1, 3)
>
>
> Thanks,
>
> Mehant Baid
>
>
Re: Review Request 20742: Allow implicit cast to be applied in both
directions for certain data types
Posted by Jinfeng Ni <jn...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20742/#review42334
-----------------------------------------------------------
exec/java-exec/src/main/java/org/apache/drill/exec/resolver/TypeCastRules.java
<https://reviews.apache.org/r/20742/#comment76102>
Here we use an identical value, if the function requires a secondary implicit cast for one of its argument. This will make the code unable to differentiate the matching of other arguments.
Let's say we have function implementation:
func (varchar, int)
func (varchar, float4)
Now, if we have a function : func(123, 8.0). For the two function implementations, the code will return the same cost, since both of them requires secondary implicit cast. Which function holder will be picked will probably depends on the order that the code adds the function holder into registry.
That is, we probably need still add the cost for other arguments to the total cost, so that we know which function holder is the best match.
- Jinfeng Ni
On April 26, 2014, 12:40 a.m., Mehant Baid wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20742/
> -----------------------------------------------------------
>
> (Updated April 26, 2014, 12:40 a.m.)
>
>
> Review request for drill, Jacques Nadeau and Jinfeng Ni.
>
>
> Bugs: DRILL-577
> https://issues.apache.org/jira/browse/DRILL-577
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> Currently implicit cast works as per the precedence rules. However based on some functions it might be useful allow the implicit cast to work in the opposite direction as specified by the precedence map.
> For example: As per the precedence rules, we can implicitly cast from VARCHAR ---> BIGINT. However for some functions (eg: substr, concat) it might be useful to implicitly cast from BIGINT ---> VARCHAR.
>
> ResolverTypePrecedence.java:
> Added a new set of secondary rules that allow for implicit cast to work in the other direction. Currently these secondary set of rules only allow to cast to VARCHAR, so we don't have cost, if we add more rules here we can add cost to determine which cast function to pick.
>
> ExpressionTreeMaterializer.java:
> Fixed bug in visitFunctionCall() that does not add an argument(length) while casting to variable width types
>
> TypeCastRules.java
> Added logic to see if we can apply implicit cast based on the secondary rules while matching a given call to a DrillFuncHolder.
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/java/org/apache/drill/exec/expr/ExpressionTreeMaterializer.java a602d82
> exec/java-exec/src/main/java/org/apache/drill/exec/resolver/ResolverTypePrecedence.java f6d83e2
> exec/java-exec/src/main/java/org/apache/drill/exec/resolver/TypeCastRules.java 3aab08f
> exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestReverseImplicitCast.java PRE-CREATION
> exec/java-exec/src/test/resources/functions/cast/two_way_implicit_cast.json PRE-CREATION
>
> Diff: https://reviews.apache.org/r/20742/diff/
>
>
> Testing
> -------
>
> Added test to check that implicit cast works both ways.
>
> 8 + '2'
> substr(10123, 1, 3)
>
>
> Thanks,
>
> Mehant Baid
>
>
Re: Review Request 20742: Allow implicit cast to be applied in both
directions for certain data types
Posted by Mehant Baid <ba...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20742/
-----------------------------------------------------------
(Updated May 7, 2014, 1:28 a.m.)
Review request for drill, Jacques Nadeau and Jinfeng Ni.
Changes
-------
Added costs to the secondary casts.
Bugs: DRILL-577
https://issues.apache.org/jira/browse/DRILL-577
Repository: drill-git
Description
-------
Currently implicit cast works as per the precedence rules. However based on some functions it might be useful allow the implicit cast to work in the opposite direction as specified by the precedence map.
For example: As per the precedence rules, we can implicitly cast from VARCHAR ---> BIGINT. However for some functions (eg: substr, concat) it might be useful to implicitly cast from BIGINT ---> VARCHAR.
ResolverTypePrecedence.java:
Added a new set of secondary rules that allow for implicit cast to work in the other direction. Currently these secondary set of rules only allow to cast to VARCHAR, so we don't have cost, if we add more rules here we can add cost to determine which cast function to pick.
ExpressionTreeMaterializer.java:
Fixed bug in visitFunctionCall() that does not add an argument(length) while casting to variable width types
TypeCastRules.java
Added logic to see if we can apply implicit cast based on the secondary rules while matching a given call to a DrillFuncHolder.
Diffs (updated)
-----
exec/java-exec/src/main/java/org/apache/drill/exec/expr/ExpressionTreeMaterializer.java a602d82
exec/java-exec/src/main/java/org/apache/drill/exec/resolver/ResolverTypePrecedence.java f6d83e2
exec/java-exec/src/main/java/org/apache/drill/exec/resolver/TypeCastRules.java 3aab08f
exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestReverseImplicitCast.java PRE-CREATION
exec/java-exec/src/test/resources/functions/cast/two_way_implicit_cast.json PRE-CREATION
Diff: https://reviews.apache.org/r/20742/diff/
Testing
-------
Added test to check that implicit cast works both ways.
8 + '2'
substr(10123, 1, 3)
Thanks,
Mehant Baid