You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by Jinfeng Ni <jn...@maprtech.com> on 2013/12/27 22:42:39 UTC
Review Request 16490: DRILL_259 : implicit cast support in drill.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16490/
-----------------------------------------------------------
Review request for drill.
Repository: drill-git
Description
-------
This patch is to provide implicit cast support in Drill.
The code change is mainly in two areas:
1. Function Resolver and related type cast rules : used to choose the best matched function implementation corresponding to a function name and its list of argument types. Type promotion would be implied, if there is no function implementation whose parameter types exactly match the argument types. Type promotion is implemented using the type precedence list. Type promotion is checked against a rule which will help implement a isCastable(from, to) method.
New files: 1) DefaultFunctionResolver.java
2) FunctionResolver
3) FunctionResolverFactor.java
4) OperatorFunctionResolver.java : reserved for future use.
5) ResolverTypePrecedence.java
6) TypeCastRules.java
7) UDFFunctionResolver.java : reserved for future use.
2. ImplicitCastBuilder : to inject implicit cast whenever necessary. After the function resolver returns a best matched function implementation, if the param type does not match the argument type, an implicit cast will be inserted on top of the argument.
- ImplicitCastBuilder uses a visitor to inject implicit cast.
- ImplicitCastBuilder will be part of ExpressionTreeMateralizer, after we materialize the expression tree.
- ExpressionValidator will be called after implicit cast is injected, in stead of after expression materialization.
- Because ImplicitCastBuilder requires a FunctionImplementationRegistry (to decide the best matched function implementation), and ImplicitCastBuilder is part of ExpressionTreeMaterializer, all the call of ExpressionTreeMaterializer is modified by adding a FunctionImplementationRegistry.
New files or modified files:
1)CastFunctionDefs.java
2)ExpressionTreeMaterializer.java
3)ImplicitCastBuilder.java
4)DrillFuncHolder.java : expose param type and size
5)FunctionImplementationRegistry.java : expose the list of function implementation.
6) other files because of the change to ExpressionTreeMaterializer.java.
Some other minor changes:
1. ArgumentValidator.ComparableArguments :
change allSame from true to false, since currently we have comparison operators defined over different types of inputs.
2. VectorUtil:
Add code to handle null value when display the result.
Diffs
-----
common/src/main/java/org/apache/drill/common/expression/ArgumentValidators.java cd30a06
common/src/main/java/org/apache/drill/common/expression/fn/CastFunctionDefs.java PRE-CREATION
exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java 5336c0e
exec/java-exec/src/main/java/org/apache/drill/exec/expr/ExpressionTreeMaterializer.java 5d576b1
exec/java-exec/src/main/java/org/apache/drill/exec/expr/ImplicitCastBuilder.java PRE-CREATION
exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFuncHolder.java e001ffe
exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionImplementationRegistry.java 5bbab76
exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/MathFunctions.java 288760b
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/AggBatch.java f652e35
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/filter/FilterRecordBatch.java fe298d6
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/MergeJoinBatch.java f3a32cd
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingRecordBatch.java fd392a3
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/orderedpartitioner/OrderedPartitionRecordBatch.java a3d1d09
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionSenderRootExec.java bc53bd2
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectRecordBatch.java 41a4c4d
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/sort/SortBatch.java 7881115
exec/java-exec/src/main/java/org/apache/drill/exec/resolver/DefaultFunctionResolver.java PRE-CREATION
exec/java-exec/src/main/java/org/apache/drill/exec/resolver/FunctionResolver.java PRE-CREATION
exec/java-exec/src/main/java/org/apache/drill/exec/resolver/FunctionResolverFactory.java PRE-CREATION
exec/java-exec/src/main/java/org/apache/drill/exec/resolver/OperatorFunctionResolver.java PRE-CREATION
exec/java-exec/src/main/java/org/apache/drill/exec/resolver/ResolverTypePrecedence.java PRE-CREATION
exec/java-exec/src/main/java/org/apache/drill/exec/resolver/TypeCastRules.java PRE-CREATION
exec/java-exec/src/main/java/org/apache/drill/exec/resolver/UDFFunctionResolver.java PRE-CREATION
exec/java-exec/src/main/java/org/apache/drill/exec/util/VectorUtil.java 8b23bcd
exec/java-exec/src/test/java/org/apache/drill/exec/expr/ExpressionTest.java 4bff5a3
exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestImplicitCastFunctions.java PRE-CREATION
exec/java-exec/src/test/java/org/apache/drill/exec/record/ExpressionTreeMaterializerTest.java 7ee1718
exec/java-exec/src/test/resources/functions/cast/testICastConstant.json PRE-CREATION
exec/java-exec/src/test/resources/functions/cast/testICastMockCol.json PRE-CREATION
exec/ref/src/test/resources/simple_plan.json c0837e6
Diff: https://reviews.apache.org/r/16490/diff/
Testing
-------
testImplicitCastFunctions.java is used for unit test. It uses two physical plans
1. testICastConstant.json : the input expression is a literal constants. eg, 1 + 1.2, etc. Also, cover the nested function cases.
2. testICastMockCol.json : the input expression is a column from mock scan operator.
Thanks,
Jinfeng Ni
Re: Review Request 16490: DRILL_259 : implicit cast support in drill.
Posted by Jason Altekruse <al...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16490/#review31194
-----------------------------------------------------------
exec/java-exec/src/main/java/org/apache/drill/exec/resolver/TypeCastRules.java
<https://reviews.apache.org/r/16490/#comment59596>
Would it be worth possibly using code generation for this section of the class? It would make understanding how the different types' casting rules differ if they were specified in a more concise format.
- Jason Altekruse
On Jan. 3, 2014, 10:57 p.m., Jinfeng Ni wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16490/
> -----------------------------------------------------------
>
> (Updated Jan. 3, 2014, 10:57 p.m.)
>
>
> Review request for drill.
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> This patch is to provide implicit cast support in Drill.
>
> The code change is mainly in two areas:
>
> 1. Function Resolver and related type cast rules : used to choose the best matched function implementation corresponding to a function name and its list of argument types. Type promotion would be implied, if there is no function implementation whose parameter types exactly match the argument types. Type promotion is implemented using the type precedence list. Type promotion is checked against a rule which will help implement a isCastable(from, to) method.
> New files: 1) DefaultFunctionResolver.java
> 2) FunctionResolver
> 3) FunctionResolverFactor.java
> 4) OperatorFunctionResolver.java : reserved for future use.
> 5) ResolverTypePrecedence.java
> 6) TypeCastRules.java
> 7) UDFFunctionResolver.java : reserved for future use.
>
> 2. ImplicitCastBuilder : to inject implicit cast whenever necessary. After the function resolver returns a best matched function implementation, if the param type does not match the argument type, an implicit cast will be inserted on top of the argument.
> - ImplicitCastBuilder uses a visitor to inject implicit cast.
> - ImplicitCastBuilder will be part of ExpressionTreeMateralizer, after we materialize the expression tree.
> - ExpressionValidator will be called after implicit cast is injected, in stead of after expression materialization.
> - Because ImplicitCastBuilder requires a FunctionImplementationRegistry (to decide the best matched function implementation), and ImplicitCastBuilder is part of ExpressionTreeMaterializer, all the call of ExpressionTreeMaterializer is modified by adding a FunctionImplementationRegistry.
>
> New files or modified files:
> 1)CastFunctionDefs.java
> 2)ExpressionTreeMaterializer.java
> 3)ImplicitCastBuilder.java
> 4)DrillFuncHolder.java : expose param type and size
> 5)FunctionImplementationRegistry.java : expose the list of function implementation.
> 6) other files because of the change to ExpressionTreeMaterializer.java.
>
> Some other minor changes:
>
> 1. ArgumentValidator.ComparableArguments :
> change allSame from true to false, since currently we have comparison operators defined over different types of inputs.
>
> 2. VectorUtil:
> Add code to handle null value when display the result.
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/drill/common/expression/ArgumentValidators.java cd30a06
> common/src/main/java/org/apache/drill/common/expression/TypedNullConstant.java PRE-CREATION
> common/src/main/java/org/apache/drill/common/expression/fn/CastFunctionDefs.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java 5336c0e
> exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java 587fe3c
> exec/java-exec/src/main/java/org/apache/drill/exec/expr/ExpressionTreeMaterializer.java 5d576b1
> exec/java-exec/src/main/java/org/apache/drill/exec/expr/ImplicitCastBuilder.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFuncHolder.java e001ffe
> exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionImplementationRegistry.java 5bbab76
> exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/MathFunctions.java 288760b
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/AggBatch.java f652e35
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/filter/FilterRecordBatch.java fe298d6
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/MergeJoinBatch.java f3a32cd
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingRecordBatch.java fd392a3
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/orderedpartitioner/OrderedPartitionRecordBatch.java a3d1d09
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionSenderRootExec.java bc53bd2
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectRecordBatch.java 41a4c4d
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/sort/SortBatch.java 7881115
> exec/java-exec/src/main/java/org/apache/drill/exec/resolver/DefaultFunctionResolver.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/resolver/FunctionResolver.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/resolver/FunctionResolverFactory.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/resolver/ResolverTypePrecedence.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/resolver/TypeCastRules.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/util/VectorUtil.java 8b23bcd
> exec/java-exec/src/test/java/org/apache/drill/exec/expr/ExpressionTest.java 4bff5a3
> exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestImplicitCastFunctions.java PRE-CREATION
> exec/java-exec/src/test/java/org/apache/drill/exec/record/ExpressionTreeMaterializerTest.java 7ee1718
> exec/java-exec/src/test/resources/functions/cast/testICastConstant.json PRE-CREATION
> exec/java-exec/src/test/resources/functions/cast/testICastMockCol.json PRE-CREATION
> exec/java-exec/src/test/resources/functions/cast/testICastNullExp.json PRE-CREATION
> exec/ref/src/test/resources/simple_plan.json c0837e6
>
> Diff: https://reviews.apache.org/r/16490/diff/
>
>
> Testing
> -------
>
> testImplicitCastFunctions.java is used for unit test. It uses two physical plans
> 1. testICastConstant.json : the input expression is a literal constants. eg, 1 + 1.2, etc. Also, cover the nested function cases.
> 2. testICastMockCol.json : the input expression is a column from mock scan operator.
>
>
> Thanks,
>
> Jinfeng Ni
>
>
Re: Review Request 16490: DRILL_259 : implicit cast support in drill.
Posted by Jacques Nadeau <ja...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16490/#review31731
-----------------------------------------------------------
Ship it!
Ship It!
- Jacques Nadeau
On Jan. 3, 2014, 10:57 p.m., Jinfeng Ni wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16490/
> -----------------------------------------------------------
>
> (Updated Jan. 3, 2014, 10:57 p.m.)
>
>
> Review request for drill.
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> This patch is to provide implicit cast support in Drill.
>
> The code change is mainly in two areas:
>
> 1. Function Resolver and related type cast rules : used to choose the best matched function implementation corresponding to a function name and its list of argument types. Type promotion would be implied, if there is no function implementation whose parameter types exactly match the argument types. Type promotion is implemented using the type precedence list. Type promotion is checked against a rule which will help implement a isCastable(from, to) method.
> New files: 1) DefaultFunctionResolver.java
> 2) FunctionResolver
> 3) FunctionResolverFactor.java
> 4) OperatorFunctionResolver.java : reserved for future use.
> 5) ResolverTypePrecedence.java
> 6) TypeCastRules.java
> 7) UDFFunctionResolver.java : reserved for future use.
>
> 2. ImplicitCastBuilder : to inject implicit cast whenever necessary. After the function resolver returns a best matched function implementation, if the param type does not match the argument type, an implicit cast will be inserted on top of the argument.
> - ImplicitCastBuilder uses a visitor to inject implicit cast.
> - ImplicitCastBuilder will be part of ExpressionTreeMateralizer, after we materialize the expression tree.
> - ExpressionValidator will be called after implicit cast is injected, in stead of after expression materialization.
> - Because ImplicitCastBuilder requires a FunctionImplementationRegistry (to decide the best matched function implementation), and ImplicitCastBuilder is part of ExpressionTreeMaterializer, all the call of ExpressionTreeMaterializer is modified by adding a FunctionImplementationRegistry.
>
> New files or modified files:
> 1)CastFunctionDefs.java
> 2)ExpressionTreeMaterializer.java
> 3)ImplicitCastBuilder.java
> 4)DrillFuncHolder.java : expose param type and size
> 5)FunctionImplementationRegistry.java : expose the list of function implementation.
> 6) other files because of the change to ExpressionTreeMaterializer.java.
>
> Some other minor changes:
>
> 1. ArgumentValidator.ComparableArguments :
> change allSame from true to false, since currently we have comparison operators defined over different types of inputs.
>
> 2. VectorUtil:
> Add code to handle null value when display the result.
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/drill/common/expression/ArgumentValidators.java cd30a06
> common/src/main/java/org/apache/drill/common/expression/TypedNullConstant.java PRE-CREATION
> common/src/main/java/org/apache/drill/common/expression/fn/CastFunctionDefs.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java 5336c0e
> exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java 587fe3c
> exec/java-exec/src/main/java/org/apache/drill/exec/expr/ExpressionTreeMaterializer.java 5d576b1
> exec/java-exec/src/main/java/org/apache/drill/exec/expr/ImplicitCastBuilder.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFuncHolder.java e001ffe
> exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionImplementationRegistry.java 5bbab76
> exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/MathFunctions.java 288760b
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/AggBatch.java f652e35
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/filter/FilterRecordBatch.java fe298d6
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/MergeJoinBatch.java f3a32cd
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingRecordBatch.java fd392a3
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/orderedpartitioner/OrderedPartitionRecordBatch.java a3d1d09
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionSenderRootExec.java bc53bd2
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectRecordBatch.java 41a4c4d
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/sort/SortBatch.java 7881115
> exec/java-exec/src/main/java/org/apache/drill/exec/resolver/DefaultFunctionResolver.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/resolver/FunctionResolver.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/resolver/FunctionResolverFactory.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/resolver/ResolverTypePrecedence.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/resolver/TypeCastRules.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/util/VectorUtil.java 8b23bcd
> exec/java-exec/src/test/java/org/apache/drill/exec/expr/ExpressionTest.java 4bff5a3
> exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestImplicitCastFunctions.java PRE-CREATION
> exec/java-exec/src/test/java/org/apache/drill/exec/record/ExpressionTreeMaterializerTest.java 7ee1718
> exec/java-exec/src/test/resources/functions/cast/testICastConstant.json PRE-CREATION
> exec/java-exec/src/test/resources/functions/cast/testICastMockCol.json PRE-CREATION
> exec/java-exec/src/test/resources/functions/cast/testICastNullExp.json PRE-CREATION
> exec/ref/src/test/resources/simple_plan.json c0837e6
>
> Diff: https://reviews.apache.org/r/16490/diff/
>
>
> Testing
> -------
>
> testImplicitCastFunctions.java is used for unit test. It uses two physical plans
> 1. testICastConstant.json : the input expression is a literal constants. eg, 1 + 1.2, etc. Also, cover the nested function cases.
> 2. testICastMockCol.json : the input expression is a column from mock scan operator.
>
>
> Thanks,
>
> Jinfeng Ni
>
>
Re: Review Request 16490: DRILL_259 : implicit cast support in drill.
Posted by Jason Altekruse <al...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16490/#review31211
-----------------------------------------------------------
exec/java-exec/src/main/java/org/apache/drill/exec/resolver/TypeCastRules.java
<https://reviews.apache.org/r/16490/#comment59611>
Should have put that through a JSON validator before posting, here is a revised version that is valid http://pastebin.com/kaZPM9Zr
- Jason Altekruse
On Jan. 3, 2014, 10:57 p.m., Jinfeng Ni wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16490/
> -----------------------------------------------------------
>
> (Updated Jan. 3, 2014, 10:57 p.m.)
>
>
> Review request for drill.
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> This patch is to provide implicit cast support in Drill.
>
> The code change is mainly in two areas:
>
> 1. Function Resolver and related type cast rules : used to choose the best matched function implementation corresponding to a function name and its list of argument types. Type promotion would be implied, if there is no function implementation whose parameter types exactly match the argument types. Type promotion is implemented using the type precedence list. Type promotion is checked against a rule which will help implement a isCastable(from, to) method.
> New files: 1) DefaultFunctionResolver.java
> 2) FunctionResolver
> 3) FunctionResolverFactor.java
> 4) OperatorFunctionResolver.java : reserved for future use.
> 5) ResolverTypePrecedence.java
> 6) TypeCastRules.java
> 7) UDFFunctionResolver.java : reserved for future use.
>
> 2. ImplicitCastBuilder : to inject implicit cast whenever necessary. After the function resolver returns a best matched function implementation, if the param type does not match the argument type, an implicit cast will be inserted on top of the argument.
> - ImplicitCastBuilder uses a visitor to inject implicit cast.
> - ImplicitCastBuilder will be part of ExpressionTreeMateralizer, after we materialize the expression tree.
> - ExpressionValidator will be called after implicit cast is injected, in stead of after expression materialization.
> - Because ImplicitCastBuilder requires a FunctionImplementationRegistry (to decide the best matched function implementation), and ImplicitCastBuilder is part of ExpressionTreeMaterializer, all the call of ExpressionTreeMaterializer is modified by adding a FunctionImplementationRegistry.
>
> New files or modified files:
> 1)CastFunctionDefs.java
> 2)ExpressionTreeMaterializer.java
> 3)ImplicitCastBuilder.java
> 4)DrillFuncHolder.java : expose param type and size
> 5)FunctionImplementationRegistry.java : expose the list of function implementation.
> 6) other files because of the change to ExpressionTreeMaterializer.java.
>
> Some other minor changes:
>
> 1. ArgumentValidator.ComparableArguments :
> change allSame from true to false, since currently we have comparison operators defined over different types of inputs.
>
> 2. VectorUtil:
> Add code to handle null value when display the result.
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/drill/common/expression/ArgumentValidators.java cd30a06
> common/src/main/java/org/apache/drill/common/expression/TypedNullConstant.java PRE-CREATION
> common/src/main/java/org/apache/drill/common/expression/fn/CastFunctionDefs.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java 5336c0e
> exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java 587fe3c
> exec/java-exec/src/main/java/org/apache/drill/exec/expr/ExpressionTreeMaterializer.java 5d576b1
> exec/java-exec/src/main/java/org/apache/drill/exec/expr/ImplicitCastBuilder.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFuncHolder.java e001ffe
> exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionImplementationRegistry.java 5bbab76
> exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/MathFunctions.java 288760b
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/AggBatch.java f652e35
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/filter/FilterRecordBatch.java fe298d6
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/MergeJoinBatch.java f3a32cd
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingRecordBatch.java fd392a3
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/orderedpartitioner/OrderedPartitionRecordBatch.java a3d1d09
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionSenderRootExec.java bc53bd2
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectRecordBatch.java 41a4c4d
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/sort/SortBatch.java 7881115
> exec/java-exec/src/main/java/org/apache/drill/exec/resolver/DefaultFunctionResolver.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/resolver/FunctionResolver.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/resolver/FunctionResolverFactory.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/resolver/ResolverTypePrecedence.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/resolver/TypeCastRules.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/util/VectorUtil.java 8b23bcd
> exec/java-exec/src/test/java/org/apache/drill/exec/expr/ExpressionTest.java 4bff5a3
> exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestImplicitCastFunctions.java PRE-CREATION
> exec/java-exec/src/test/java/org/apache/drill/exec/record/ExpressionTreeMaterializerTest.java 7ee1718
> exec/java-exec/src/test/resources/functions/cast/testICastConstant.json PRE-CREATION
> exec/java-exec/src/test/resources/functions/cast/testICastMockCol.json PRE-CREATION
> exec/java-exec/src/test/resources/functions/cast/testICastNullExp.json PRE-CREATION
> exec/ref/src/test/resources/simple_plan.json c0837e6
>
> Diff: https://reviews.apache.org/r/16490/diff/
>
>
> Testing
> -------
>
> testImplicitCastFunctions.java is used for unit test. It uses two physical plans
> 1. testICastConstant.json : the input expression is a literal constants. eg, 1 + 1.2, etc. Also, cover the nested function cases.
> 2. testICastMockCol.json : the input expression is a column from mock scan operator.
>
>
> Thanks,
>
> Jinfeng Ni
>
>
Re: Review Request 16490: DRILL_259 : implicit cast support in drill.
Posted by Jason Altekruse <al...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16490/#review31210
-----------------------------------------------------------
exec/java-exec/src/main/java/org/apache/drill/exec/resolver/TypeCastRules.java
<https://reviews.apache.org/r/16490/#comment59610>
I'm thinking something like this, it would give a nice overview to anyone curious about the differences in what can be casted and what cannot. Doesn't look the best because of the wordwrap, but if you paste it into a local editor to see that it is aligned for easy reading. http://pastebin.com/aysWxVFx
- Jason Altekruse
On Jan. 3, 2014, 10:57 p.m., Jinfeng Ni wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16490/
> -----------------------------------------------------------
>
> (Updated Jan. 3, 2014, 10:57 p.m.)
>
>
> Review request for drill.
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> This patch is to provide implicit cast support in Drill.
>
> The code change is mainly in two areas:
>
> 1. Function Resolver and related type cast rules : used to choose the best matched function implementation corresponding to a function name and its list of argument types. Type promotion would be implied, if there is no function implementation whose parameter types exactly match the argument types. Type promotion is implemented using the type precedence list. Type promotion is checked against a rule which will help implement a isCastable(from, to) method.
> New files: 1) DefaultFunctionResolver.java
> 2) FunctionResolver
> 3) FunctionResolverFactor.java
> 4) OperatorFunctionResolver.java : reserved for future use.
> 5) ResolverTypePrecedence.java
> 6) TypeCastRules.java
> 7) UDFFunctionResolver.java : reserved for future use.
>
> 2. ImplicitCastBuilder : to inject implicit cast whenever necessary. After the function resolver returns a best matched function implementation, if the param type does not match the argument type, an implicit cast will be inserted on top of the argument.
> - ImplicitCastBuilder uses a visitor to inject implicit cast.
> - ImplicitCastBuilder will be part of ExpressionTreeMateralizer, after we materialize the expression tree.
> - ExpressionValidator will be called after implicit cast is injected, in stead of after expression materialization.
> - Because ImplicitCastBuilder requires a FunctionImplementationRegistry (to decide the best matched function implementation), and ImplicitCastBuilder is part of ExpressionTreeMaterializer, all the call of ExpressionTreeMaterializer is modified by adding a FunctionImplementationRegistry.
>
> New files or modified files:
> 1)CastFunctionDefs.java
> 2)ExpressionTreeMaterializer.java
> 3)ImplicitCastBuilder.java
> 4)DrillFuncHolder.java : expose param type and size
> 5)FunctionImplementationRegistry.java : expose the list of function implementation.
> 6) other files because of the change to ExpressionTreeMaterializer.java.
>
> Some other minor changes:
>
> 1. ArgumentValidator.ComparableArguments :
> change allSame from true to false, since currently we have comparison operators defined over different types of inputs.
>
> 2. VectorUtil:
> Add code to handle null value when display the result.
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/drill/common/expression/ArgumentValidators.java cd30a06
> common/src/main/java/org/apache/drill/common/expression/TypedNullConstant.java PRE-CREATION
> common/src/main/java/org/apache/drill/common/expression/fn/CastFunctionDefs.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java 5336c0e
> exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java 587fe3c
> exec/java-exec/src/main/java/org/apache/drill/exec/expr/ExpressionTreeMaterializer.java 5d576b1
> exec/java-exec/src/main/java/org/apache/drill/exec/expr/ImplicitCastBuilder.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFuncHolder.java e001ffe
> exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionImplementationRegistry.java 5bbab76
> exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/MathFunctions.java 288760b
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/AggBatch.java f652e35
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/filter/FilterRecordBatch.java fe298d6
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/MergeJoinBatch.java f3a32cd
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingRecordBatch.java fd392a3
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/orderedpartitioner/OrderedPartitionRecordBatch.java a3d1d09
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionSenderRootExec.java bc53bd2
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectRecordBatch.java 41a4c4d
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/sort/SortBatch.java 7881115
> exec/java-exec/src/main/java/org/apache/drill/exec/resolver/DefaultFunctionResolver.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/resolver/FunctionResolver.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/resolver/FunctionResolverFactory.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/resolver/ResolverTypePrecedence.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/resolver/TypeCastRules.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/util/VectorUtil.java 8b23bcd
> exec/java-exec/src/test/java/org/apache/drill/exec/expr/ExpressionTest.java 4bff5a3
> exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestImplicitCastFunctions.java PRE-CREATION
> exec/java-exec/src/test/java/org/apache/drill/exec/record/ExpressionTreeMaterializerTest.java 7ee1718
> exec/java-exec/src/test/resources/functions/cast/testICastConstant.json PRE-CREATION
> exec/java-exec/src/test/resources/functions/cast/testICastMockCol.json PRE-CREATION
> exec/java-exec/src/test/resources/functions/cast/testICastNullExp.json PRE-CREATION
> exec/ref/src/test/resources/simple_plan.json c0837e6
>
> Diff: https://reviews.apache.org/r/16490/diff/
>
>
> Testing
> -------
>
> testImplicitCastFunctions.java is used for unit test. It uses two physical plans
> 1. testICastConstant.json : the input expression is a literal constants. eg, 1 + 1.2, etc. Also, cover the nested function cases.
> 2. testICastMockCol.json : the input expression is a column from mock scan operator.
>
>
> Thanks,
>
> Jinfeng Ni
>
>
Re: Review Request 16490: DRILL_259 : implicit cast support in drill.
Posted by Jinfeng Ni <jn...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16490/
-----------------------------------------------------------
(Updated Jan. 3, 2014, 2:57 p.m.)
Review request for drill.
Changes
-------
Modify the code to address review comments.
Repository: drill-git
Description
-------
This patch is to provide implicit cast support in Drill.
The code change is mainly in two areas:
1. Function Resolver and related type cast rules : used to choose the best matched function implementation corresponding to a function name and its list of argument types. Type promotion would be implied, if there is no function implementation whose parameter types exactly match the argument types. Type promotion is implemented using the type precedence list. Type promotion is checked against a rule which will help implement a isCastable(from, to) method.
New files: 1) DefaultFunctionResolver.java
2) FunctionResolver
3) FunctionResolverFactor.java
4) OperatorFunctionResolver.java : reserved for future use.
5) ResolverTypePrecedence.java
6) TypeCastRules.java
7) UDFFunctionResolver.java : reserved for future use.
2. ImplicitCastBuilder : to inject implicit cast whenever necessary. After the function resolver returns a best matched function implementation, if the param type does not match the argument type, an implicit cast will be inserted on top of the argument.
- ImplicitCastBuilder uses a visitor to inject implicit cast.
- ImplicitCastBuilder will be part of ExpressionTreeMateralizer, after we materialize the expression tree.
- ExpressionValidator will be called after implicit cast is injected, in stead of after expression materialization.
- Because ImplicitCastBuilder requires a FunctionImplementationRegistry (to decide the best matched function implementation), and ImplicitCastBuilder is part of ExpressionTreeMaterializer, all the call of ExpressionTreeMaterializer is modified by adding a FunctionImplementationRegistry.
New files or modified files:
1)CastFunctionDefs.java
2)ExpressionTreeMaterializer.java
3)ImplicitCastBuilder.java
4)DrillFuncHolder.java : expose param type and size
5)FunctionImplementationRegistry.java : expose the list of function implementation.
6) other files because of the change to ExpressionTreeMaterializer.java.
Some other minor changes:
1. ArgumentValidator.ComparableArguments :
change allSame from true to false, since currently we have comparison operators defined over different types of inputs.
2. VectorUtil:
Add code to handle null value when display the result.
Diffs (updated)
-----
common/src/main/java/org/apache/drill/common/expression/ArgumentValidators.java cd30a06
common/src/main/java/org/apache/drill/common/expression/TypedNullConstant.java PRE-CREATION
common/src/main/java/org/apache/drill/common/expression/fn/CastFunctionDefs.java PRE-CREATION
exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java 5336c0e
exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java 587fe3c
exec/java-exec/src/main/java/org/apache/drill/exec/expr/ExpressionTreeMaterializer.java 5d576b1
exec/java-exec/src/main/java/org/apache/drill/exec/expr/ImplicitCastBuilder.java PRE-CREATION
exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFuncHolder.java e001ffe
exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionImplementationRegistry.java 5bbab76
exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/MathFunctions.java 288760b
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/AggBatch.java f652e35
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/filter/FilterRecordBatch.java fe298d6
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/MergeJoinBatch.java f3a32cd
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingRecordBatch.java fd392a3
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/orderedpartitioner/OrderedPartitionRecordBatch.java a3d1d09
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionSenderRootExec.java bc53bd2
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectRecordBatch.java 41a4c4d
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/sort/SortBatch.java 7881115
exec/java-exec/src/main/java/org/apache/drill/exec/resolver/DefaultFunctionResolver.java PRE-CREATION
exec/java-exec/src/main/java/org/apache/drill/exec/resolver/FunctionResolver.java PRE-CREATION
exec/java-exec/src/main/java/org/apache/drill/exec/resolver/FunctionResolverFactory.java PRE-CREATION
exec/java-exec/src/main/java/org/apache/drill/exec/resolver/ResolverTypePrecedence.java PRE-CREATION
exec/java-exec/src/main/java/org/apache/drill/exec/resolver/TypeCastRules.java PRE-CREATION
exec/java-exec/src/main/java/org/apache/drill/exec/util/VectorUtil.java 8b23bcd
exec/java-exec/src/test/java/org/apache/drill/exec/expr/ExpressionTest.java 4bff5a3
exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestImplicitCastFunctions.java PRE-CREATION
exec/java-exec/src/test/java/org/apache/drill/exec/record/ExpressionTreeMaterializerTest.java 7ee1718
exec/java-exec/src/test/resources/functions/cast/testICastConstant.json PRE-CREATION
exec/java-exec/src/test/resources/functions/cast/testICastMockCol.json PRE-CREATION
exec/java-exec/src/test/resources/functions/cast/testICastNullExp.json PRE-CREATION
exec/ref/src/test/resources/simple_plan.json c0837e6
Diff: https://reviews.apache.org/r/16490/diff/
Testing
-------
testImplicitCastFunctions.java is used for unit test. It uses two physical plans
1. testICastConstant.json : the input expression is a literal constants. eg, 1 + 1.2, etc. Also, cover the nested function cases.
2. testICastMockCol.json : the input expression is a column from mock scan operator.
Thanks,
Jinfeng Ni
Re: Review Request 16490: DRILL_259 : implicit cast support in drill.
Posted by Jinfeng Ni <jn...@maprtech.com>.
> On Jan. 3, 2014, 10:56 a.m., Timothy Chen wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/expr/ImplicitCastBuilder.java, line 100
> > <https://reviews.apache.org/r/16490/diff/2/?file=406736#file406736line100>
> >
> > "exchangable"
Modified.
> On Jan. 3, 2014, 10:56 a.m., Timothy Chen wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/resolver/DefaultFunctionResolver.java, line 29
> > <https://reviews.apache.org/r/16490/diff/2/?file=406748#file406748line29>
> >
> > this is quite some funky formating :)
Modified.
> On Jan. 3, 2014, 10:56 a.m., Timothy Chen wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/resolver/FunctionResolverFactory.java, line 26
> > <https://reviews.apache.org/r/16490/diff/2/?file=406750#file406750line26>
> >
> > Remove?
> >
> > And also I don't see where OperationFunctionResolver is being used, so the default or all function resolving is using the default, which looks for least cost cast right?
You are right. Currently, the code always uses DefaultFunctionResolver, which chooses cast based on the criteria of lowest cost (defined using the type precedence list); OperatorFunctionResolver etc is not used at all.
However, we might need implement a different resolver for different type of operator/function, in case the rule imposed from type precedence list is not enough for one particular operator/function.
I can remove those currently un-used resolvers, if you or other think they are not needed.
> On Jan. 3, 2014, 10:56 a.m., Timothy Chen wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/resolver/TypeCastRules.java, line 272
> > <https://reviews.apache.org/r/16490/diff/2/?file=406753#file406753line272>
> >
> > This uses epoch time?
Yash might have a better idea, since he put those rules together, probably from Optiq.
>From what I understand, yes, it will convert epoch time into datetime/timestamp. For instance, postgres has function to_timestamp(double precision). Whether we support such function probably depends on how we implement date/time type in Drill.
> On Jan. 3, 2014, 10:56 a.m., Timothy Chen wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/resolver/TypeCastRules.java, line 549
> > <https://reviews.apache.org/r/16490/diff/2/?file=406753#file406753line549>
> >
> > Remove?
> >
> > And I don't see how dmPrecedenceMap is ever being used?
Originally, I tried to use dmPrecendenceMap to force a rule that REQUIRED value could be promoted to OPTIONAL value, but not the opposite. However, current Drill code (Types.softEquals) treat REQUIRED and OPTIONAL as inter-changeable, and hence dmPrecendenceMap is of no use currently.
I'll remove it from the code.
- Jinfeng
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16490/#review31106
-----------------------------------------------------------
On Dec. 30, 2013, 5:36 p.m., Jinfeng Ni wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16490/
> -----------------------------------------------------------
>
> (Updated Dec. 30, 2013, 5:36 p.m.)
>
>
> Review request for drill.
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> This patch is to provide implicit cast support in Drill.
>
> The code change is mainly in two areas:
>
> 1. Function Resolver and related type cast rules : used to choose the best matched function implementation corresponding to a function name and its list of argument types. Type promotion would be implied, if there is no function implementation whose parameter types exactly match the argument types. Type promotion is implemented using the type precedence list. Type promotion is checked against a rule which will help implement a isCastable(from, to) method.
> New files: 1) DefaultFunctionResolver.java
> 2) FunctionResolver
> 3) FunctionResolverFactor.java
> 4) OperatorFunctionResolver.java : reserved for future use.
> 5) ResolverTypePrecedence.java
> 6) TypeCastRules.java
> 7) UDFFunctionResolver.java : reserved for future use.
>
> 2. ImplicitCastBuilder : to inject implicit cast whenever necessary. After the function resolver returns a best matched function implementation, if the param type does not match the argument type, an implicit cast will be inserted on top of the argument.
> - ImplicitCastBuilder uses a visitor to inject implicit cast.
> - ImplicitCastBuilder will be part of ExpressionTreeMateralizer, after we materialize the expression tree.
> - ExpressionValidator will be called after implicit cast is injected, in stead of after expression materialization.
> - Because ImplicitCastBuilder requires a FunctionImplementationRegistry (to decide the best matched function implementation), and ImplicitCastBuilder is part of ExpressionTreeMaterializer, all the call of ExpressionTreeMaterializer is modified by adding a FunctionImplementationRegistry.
>
> New files or modified files:
> 1)CastFunctionDefs.java
> 2)ExpressionTreeMaterializer.java
> 3)ImplicitCastBuilder.java
> 4)DrillFuncHolder.java : expose param type and size
> 5)FunctionImplementationRegistry.java : expose the list of function implementation.
> 6) other files because of the change to ExpressionTreeMaterializer.java.
>
> Some other minor changes:
>
> 1. ArgumentValidator.ComparableArguments :
> change allSame from true to false, since currently we have comparison operators defined over different types of inputs.
>
> 2. VectorUtil:
> Add code to handle null value when display the result.
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/drill/common/expression/ArgumentValidators.java cd30a06
> common/src/main/java/org/apache/drill/common/expression/TypedNullConstant.java PRE-CREATION
> common/src/main/java/org/apache/drill/common/expression/fn/CastFunctionDefs.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java 5336c0e
> exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java 587fe3c
> exec/java-exec/src/main/java/org/apache/drill/exec/expr/ExpressionTreeMaterializer.java 5d576b1
> exec/java-exec/src/main/java/org/apache/drill/exec/expr/ImplicitCastBuilder.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFuncHolder.java e001ffe
> exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionImplementationRegistry.java 5bbab76
> exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/MathFunctions.java 288760b
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/AggBatch.java f652e35
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/filter/FilterRecordBatch.java fe298d6
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/MergeJoinBatch.java f3a32cd
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingRecordBatch.java fd392a3
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/orderedpartitioner/OrderedPartitionRecordBatch.java a3d1d09
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionSenderRootExec.java bc53bd2
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectRecordBatch.java 41a4c4d
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/sort/SortBatch.java 7881115
> exec/java-exec/src/main/java/org/apache/drill/exec/resolver/DefaultFunctionResolver.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/resolver/FunctionResolver.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/resolver/FunctionResolverFactory.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/resolver/OperatorFunctionResolver.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/resolver/ResolverTypePrecedence.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/resolver/TypeCastRules.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/resolver/UDFFunctionResolver.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/util/VectorUtil.java 8b23bcd
> exec/java-exec/src/test/java/org/apache/drill/exec/expr/ExpressionTest.java 4bff5a3
> exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestImplicitCastFunctions.java PRE-CREATION
> exec/java-exec/src/test/java/org/apache/drill/exec/record/ExpressionTreeMaterializerTest.java 7ee1718
> exec/java-exec/src/test/resources/functions/cast/testICastConstant.json PRE-CREATION
> exec/java-exec/src/test/resources/functions/cast/testICastMockCol.json PRE-CREATION
> exec/java-exec/src/test/resources/functions/cast/testICastNullExp.json PRE-CREATION
> exec/ref/src/test/resources/simple_plan.json c0837e6
>
> Diff: https://reviews.apache.org/r/16490/diff/
>
>
> Testing
> -------
>
> testImplicitCastFunctions.java is used for unit test. It uses two physical plans
> 1. testICastConstant.json : the input expression is a literal constants. eg, 1 + 1.2, etc. Also, cover the nested function cases.
> 2. testICastMockCol.json : the input expression is a column from mock scan operator.
>
>
> Thanks,
>
> Jinfeng Ni
>
>
Re: Review Request 16490: DRILL_259 : implicit cast support in drill.
Posted by Yash Sharma <ya...@gmail.com>.
> On Jan. 3, 2014, 6:56 p.m., Timothy Chen wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/resolver/TypeCastRules.java, line 272
> > <https://reviews.apache.org/r/16490/diff/2/?file=406753#file406753line272>
> >
> > This uses epoch time?
>
> Jinfeng Ni wrote:
> Yash might have a better idea, since he put those rules together, probably from Optiq.
>
> From what I understand, yes, it will convert epoch time into datetime/timestamp. For instance, postgres has function to_timestamp(double precision). Whether we support such function probably depends on how we implement date/time type in Drill.
I did not think of epoch time conversions while creating the rule set. Created it similar to that being used in Optiq with Drill's datatypes. It would be a good thing to have and we can discuss on how we can support the conversion in drill.
- Yash
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16490/#review31106
-----------------------------------------------------------
On Jan. 3, 2014, 10:57 p.m., Jinfeng Ni wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16490/
> -----------------------------------------------------------
>
> (Updated Jan. 3, 2014, 10:57 p.m.)
>
>
> Review request for drill.
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> This patch is to provide implicit cast support in Drill.
>
> The code change is mainly in two areas:
>
> 1. Function Resolver and related type cast rules : used to choose the best matched function implementation corresponding to a function name and its list of argument types. Type promotion would be implied, if there is no function implementation whose parameter types exactly match the argument types. Type promotion is implemented using the type precedence list. Type promotion is checked against a rule which will help implement a isCastable(from, to) method.
> New files: 1) DefaultFunctionResolver.java
> 2) FunctionResolver
> 3) FunctionResolverFactor.java
> 4) OperatorFunctionResolver.java : reserved for future use.
> 5) ResolverTypePrecedence.java
> 6) TypeCastRules.java
> 7) UDFFunctionResolver.java : reserved for future use.
>
> 2. ImplicitCastBuilder : to inject implicit cast whenever necessary. After the function resolver returns a best matched function implementation, if the param type does not match the argument type, an implicit cast will be inserted on top of the argument.
> - ImplicitCastBuilder uses a visitor to inject implicit cast.
> - ImplicitCastBuilder will be part of ExpressionTreeMateralizer, after we materialize the expression tree.
> - ExpressionValidator will be called after implicit cast is injected, in stead of after expression materialization.
> - Because ImplicitCastBuilder requires a FunctionImplementationRegistry (to decide the best matched function implementation), and ImplicitCastBuilder is part of ExpressionTreeMaterializer, all the call of ExpressionTreeMaterializer is modified by adding a FunctionImplementationRegistry.
>
> New files or modified files:
> 1)CastFunctionDefs.java
> 2)ExpressionTreeMaterializer.java
> 3)ImplicitCastBuilder.java
> 4)DrillFuncHolder.java : expose param type and size
> 5)FunctionImplementationRegistry.java : expose the list of function implementation.
> 6) other files because of the change to ExpressionTreeMaterializer.java.
>
> Some other minor changes:
>
> 1. ArgumentValidator.ComparableArguments :
> change allSame from true to false, since currently we have comparison operators defined over different types of inputs.
>
> 2. VectorUtil:
> Add code to handle null value when display the result.
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/drill/common/expression/ArgumentValidators.java cd30a06
> common/src/main/java/org/apache/drill/common/expression/TypedNullConstant.java PRE-CREATION
> common/src/main/java/org/apache/drill/common/expression/fn/CastFunctionDefs.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java 5336c0e
> exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java 587fe3c
> exec/java-exec/src/main/java/org/apache/drill/exec/expr/ExpressionTreeMaterializer.java 5d576b1
> exec/java-exec/src/main/java/org/apache/drill/exec/expr/ImplicitCastBuilder.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFuncHolder.java e001ffe
> exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionImplementationRegistry.java 5bbab76
> exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/MathFunctions.java 288760b
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/AggBatch.java f652e35
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/filter/FilterRecordBatch.java fe298d6
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/MergeJoinBatch.java f3a32cd
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingRecordBatch.java fd392a3
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/orderedpartitioner/OrderedPartitionRecordBatch.java a3d1d09
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionSenderRootExec.java bc53bd2
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectRecordBatch.java 41a4c4d
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/sort/SortBatch.java 7881115
> exec/java-exec/src/main/java/org/apache/drill/exec/resolver/DefaultFunctionResolver.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/resolver/FunctionResolver.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/resolver/FunctionResolverFactory.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/resolver/ResolverTypePrecedence.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/resolver/TypeCastRules.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/util/VectorUtil.java 8b23bcd
> exec/java-exec/src/test/java/org/apache/drill/exec/expr/ExpressionTest.java 4bff5a3
> exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestImplicitCastFunctions.java PRE-CREATION
> exec/java-exec/src/test/java/org/apache/drill/exec/record/ExpressionTreeMaterializerTest.java 7ee1718
> exec/java-exec/src/test/resources/functions/cast/testICastConstant.json PRE-CREATION
> exec/java-exec/src/test/resources/functions/cast/testICastMockCol.json PRE-CREATION
> exec/java-exec/src/test/resources/functions/cast/testICastNullExp.json PRE-CREATION
> exec/ref/src/test/resources/simple_plan.json c0837e6
>
> Diff: https://reviews.apache.org/r/16490/diff/
>
>
> Testing
> -------
>
> testImplicitCastFunctions.java is used for unit test. It uses two physical plans
> 1. testICastConstant.json : the input expression is a literal constants. eg, 1 + 1.2, etc. Also, cover the nested function cases.
> 2. testICastMockCol.json : the input expression is a column from mock scan operator.
>
>
> Thanks,
>
> Jinfeng Ni
>
>
Re: Review Request 16490: DRILL_259 : implicit cast support in drill.
Posted by Jinfeng Ni <jn...@maprtech.com>.
> On Jan. 3, 2014, 10:56 a.m., Timothy Chen wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/expr/ExpressionTreeMaterializer.java, line 81
> > <https://reviews.apache.org/r/16490/diff/2/?file=406735#file406735line81>
> >
> > What's the reason for removing the validate?
validate is moved from ExpressionTreeMaterializer to ImplicitCastBuilder :
Step 1: Materialization
Step 2: Inject implicit cast
Step 3: Validate.
If we do validate step between step 1 and step 2 ( before inject implicit cast), then some function call, which requires implicit cast, will fail at it's ArgumentValidator.validateArguments() call. That's why validate is moved to step 3.
- Jinfeng
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16490/#review31106
-----------------------------------------------------------
On Dec. 30, 2013, 5:36 p.m., Jinfeng Ni wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16490/
> -----------------------------------------------------------
>
> (Updated Dec. 30, 2013, 5:36 p.m.)
>
>
> Review request for drill.
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> This patch is to provide implicit cast support in Drill.
>
> The code change is mainly in two areas:
>
> 1. Function Resolver and related type cast rules : used to choose the best matched function implementation corresponding to a function name and its list of argument types. Type promotion would be implied, if there is no function implementation whose parameter types exactly match the argument types. Type promotion is implemented using the type precedence list. Type promotion is checked against a rule which will help implement a isCastable(from, to) method.
> New files: 1) DefaultFunctionResolver.java
> 2) FunctionResolver
> 3) FunctionResolverFactor.java
> 4) OperatorFunctionResolver.java : reserved for future use.
> 5) ResolverTypePrecedence.java
> 6) TypeCastRules.java
> 7) UDFFunctionResolver.java : reserved for future use.
>
> 2. ImplicitCastBuilder : to inject implicit cast whenever necessary. After the function resolver returns a best matched function implementation, if the param type does not match the argument type, an implicit cast will be inserted on top of the argument.
> - ImplicitCastBuilder uses a visitor to inject implicit cast.
> - ImplicitCastBuilder will be part of ExpressionTreeMateralizer, after we materialize the expression tree.
> - ExpressionValidator will be called after implicit cast is injected, in stead of after expression materialization.
> - Because ImplicitCastBuilder requires a FunctionImplementationRegistry (to decide the best matched function implementation), and ImplicitCastBuilder is part of ExpressionTreeMaterializer, all the call of ExpressionTreeMaterializer is modified by adding a FunctionImplementationRegistry.
>
> New files or modified files:
> 1)CastFunctionDefs.java
> 2)ExpressionTreeMaterializer.java
> 3)ImplicitCastBuilder.java
> 4)DrillFuncHolder.java : expose param type and size
> 5)FunctionImplementationRegistry.java : expose the list of function implementation.
> 6) other files because of the change to ExpressionTreeMaterializer.java.
>
> Some other minor changes:
>
> 1. ArgumentValidator.ComparableArguments :
> change allSame from true to false, since currently we have comparison operators defined over different types of inputs.
>
> 2. VectorUtil:
> Add code to handle null value when display the result.
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/drill/common/expression/ArgumentValidators.java cd30a06
> common/src/main/java/org/apache/drill/common/expression/TypedNullConstant.java PRE-CREATION
> common/src/main/java/org/apache/drill/common/expression/fn/CastFunctionDefs.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java 5336c0e
> exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java 587fe3c
> exec/java-exec/src/main/java/org/apache/drill/exec/expr/ExpressionTreeMaterializer.java 5d576b1
> exec/java-exec/src/main/java/org/apache/drill/exec/expr/ImplicitCastBuilder.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFuncHolder.java e001ffe
> exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionImplementationRegistry.java 5bbab76
> exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/MathFunctions.java 288760b
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/AggBatch.java f652e35
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/filter/FilterRecordBatch.java fe298d6
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/MergeJoinBatch.java f3a32cd
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingRecordBatch.java fd392a3
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/orderedpartitioner/OrderedPartitionRecordBatch.java a3d1d09
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionSenderRootExec.java bc53bd2
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectRecordBatch.java 41a4c4d
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/sort/SortBatch.java 7881115
> exec/java-exec/src/main/java/org/apache/drill/exec/resolver/DefaultFunctionResolver.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/resolver/FunctionResolver.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/resolver/FunctionResolverFactory.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/resolver/OperatorFunctionResolver.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/resolver/ResolverTypePrecedence.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/resolver/TypeCastRules.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/resolver/UDFFunctionResolver.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/util/VectorUtil.java 8b23bcd
> exec/java-exec/src/test/java/org/apache/drill/exec/expr/ExpressionTest.java 4bff5a3
> exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestImplicitCastFunctions.java PRE-CREATION
> exec/java-exec/src/test/java/org/apache/drill/exec/record/ExpressionTreeMaterializerTest.java 7ee1718
> exec/java-exec/src/test/resources/functions/cast/testICastConstant.json PRE-CREATION
> exec/java-exec/src/test/resources/functions/cast/testICastMockCol.json PRE-CREATION
> exec/java-exec/src/test/resources/functions/cast/testICastNullExp.json PRE-CREATION
> exec/ref/src/test/resources/simple_plan.json c0837e6
>
> Diff: https://reviews.apache.org/r/16490/diff/
>
>
> Testing
> -------
>
> testImplicitCastFunctions.java is used for unit test. It uses two physical plans
> 1. testICastConstant.json : the input expression is a literal constants. eg, 1 + 1.2, etc. Also, cover the nested function cases.
> 2. testICastMockCol.json : the input expression is a column from mock scan operator.
>
>
> Thanks,
>
> Jinfeng Ni
>
>
Re: Review Request 16490: DRILL_259 : implicit cast support in drill.
Posted by Timothy Chen <tn...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16490/#review31106
-----------------------------------------------------------
exec/java-exec/src/main/java/org/apache/drill/exec/expr/ExpressionTreeMaterializer.java
<https://reviews.apache.org/r/16490/#comment59457>
What's the reason for removing the validate?
exec/java-exec/src/main/java/org/apache/drill/exec/expr/ImplicitCastBuilder.java
<https://reviews.apache.org/r/16490/#comment59503>
"exchangable"
exec/java-exec/src/main/java/org/apache/drill/exec/resolver/DefaultFunctionResolver.java
<https://reviews.apache.org/r/16490/#comment59504>
this is quite some funky formating :)
exec/java-exec/src/main/java/org/apache/drill/exec/resolver/FunctionResolverFactory.java
<https://reviews.apache.org/r/16490/#comment59505>
Remove?
And also I don't see where OperationFunctionResolver is being used, so the default or all function resolving is using the default, which looks for least cost cast right?
exec/java-exec/src/main/java/org/apache/drill/exec/resolver/TypeCastRules.java
<https://reviews.apache.org/r/16490/#comment59507>
This uses epoch time?
exec/java-exec/src/main/java/org/apache/drill/exec/resolver/TypeCastRules.java
<https://reviews.apache.org/r/16490/#comment59510>
Remove?
And I don't see how dmPrecedenceMap is ever being used?
- Timothy Chen
On Dec. 31, 2013, 1:36 a.m., Jinfeng Ni wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16490/
> -----------------------------------------------------------
>
> (Updated Dec. 31, 2013, 1:36 a.m.)
>
>
> Review request for drill.
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> This patch is to provide implicit cast support in Drill.
>
> The code change is mainly in two areas:
>
> 1. Function Resolver and related type cast rules : used to choose the best matched function implementation corresponding to a function name and its list of argument types. Type promotion would be implied, if there is no function implementation whose parameter types exactly match the argument types. Type promotion is implemented using the type precedence list. Type promotion is checked against a rule which will help implement a isCastable(from, to) method.
> New files: 1) DefaultFunctionResolver.java
> 2) FunctionResolver
> 3) FunctionResolverFactor.java
> 4) OperatorFunctionResolver.java : reserved for future use.
> 5) ResolverTypePrecedence.java
> 6) TypeCastRules.java
> 7) UDFFunctionResolver.java : reserved for future use.
>
> 2. ImplicitCastBuilder : to inject implicit cast whenever necessary. After the function resolver returns a best matched function implementation, if the param type does not match the argument type, an implicit cast will be inserted on top of the argument.
> - ImplicitCastBuilder uses a visitor to inject implicit cast.
> - ImplicitCastBuilder will be part of ExpressionTreeMateralizer, after we materialize the expression tree.
> - ExpressionValidator will be called after implicit cast is injected, in stead of after expression materialization.
> - Because ImplicitCastBuilder requires a FunctionImplementationRegistry (to decide the best matched function implementation), and ImplicitCastBuilder is part of ExpressionTreeMaterializer, all the call of ExpressionTreeMaterializer is modified by adding a FunctionImplementationRegistry.
>
> New files or modified files:
> 1)CastFunctionDefs.java
> 2)ExpressionTreeMaterializer.java
> 3)ImplicitCastBuilder.java
> 4)DrillFuncHolder.java : expose param type and size
> 5)FunctionImplementationRegistry.java : expose the list of function implementation.
> 6) other files because of the change to ExpressionTreeMaterializer.java.
>
> Some other minor changes:
>
> 1. ArgumentValidator.ComparableArguments :
> change allSame from true to false, since currently we have comparison operators defined over different types of inputs.
>
> 2. VectorUtil:
> Add code to handle null value when display the result.
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/drill/common/expression/ArgumentValidators.java cd30a06
> common/src/main/java/org/apache/drill/common/expression/TypedNullConstant.java PRE-CREATION
> common/src/main/java/org/apache/drill/common/expression/fn/CastFunctionDefs.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java 5336c0e
> exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java 587fe3c
> exec/java-exec/src/main/java/org/apache/drill/exec/expr/ExpressionTreeMaterializer.java 5d576b1
> exec/java-exec/src/main/java/org/apache/drill/exec/expr/ImplicitCastBuilder.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFuncHolder.java e001ffe
> exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionImplementationRegistry.java 5bbab76
> exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/MathFunctions.java 288760b
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/AggBatch.java f652e35
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/filter/FilterRecordBatch.java fe298d6
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/MergeJoinBatch.java f3a32cd
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingRecordBatch.java fd392a3
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/orderedpartitioner/OrderedPartitionRecordBatch.java a3d1d09
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionSenderRootExec.java bc53bd2
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectRecordBatch.java 41a4c4d
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/sort/SortBatch.java 7881115
> exec/java-exec/src/main/java/org/apache/drill/exec/resolver/DefaultFunctionResolver.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/resolver/FunctionResolver.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/resolver/FunctionResolverFactory.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/resolver/OperatorFunctionResolver.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/resolver/ResolverTypePrecedence.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/resolver/TypeCastRules.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/resolver/UDFFunctionResolver.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/util/VectorUtil.java 8b23bcd
> exec/java-exec/src/test/java/org/apache/drill/exec/expr/ExpressionTest.java 4bff5a3
> exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestImplicitCastFunctions.java PRE-CREATION
> exec/java-exec/src/test/java/org/apache/drill/exec/record/ExpressionTreeMaterializerTest.java 7ee1718
> exec/java-exec/src/test/resources/functions/cast/testICastConstant.json PRE-CREATION
> exec/java-exec/src/test/resources/functions/cast/testICastMockCol.json PRE-CREATION
> exec/java-exec/src/test/resources/functions/cast/testICastNullExp.json PRE-CREATION
> exec/ref/src/test/resources/simple_plan.json c0837e6
>
> Diff: https://reviews.apache.org/r/16490/diff/
>
>
> Testing
> -------
>
> testImplicitCastFunctions.java is used for unit test. It uses two physical plans
> 1. testICastConstant.json : the input expression is a literal constants. eg, 1 + 1.2, etc. Also, cover the nested function cases.
> 2. testICastMockCol.json : the input expression is a column from mock scan operator.
>
>
> Thanks,
>
> Jinfeng Ni
>
>
Re: Review Request 16490: DRILL_259 : implicit cast support in drill.
Posted by Jinfeng Ni <jn...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16490/
-----------------------------------------------------------
(Updated Dec. 30, 2013, 5:36 p.m.)
Review request for drill.
Changes
-------
Add code to handle NullExpression : isnull(unknownCol) should return true.
Internally, unknowCol will be represented by NullExpression.INSTANCE. When the code is doing function resolution, if 1) the input expression is NullExpression.INSTANCE and 2)the function implementation allows null as input, then, the code will replace the NullExpression.INSTANCE with a TypedNullConstant. This TypedNullConstant will ensure the corresponding ValueHolder's isSet=0.
New file or modified file:
1. TypedNullConstant.java : to represent a null constant for one particular type.
2. EvaluationVisitor : when see TypedNullConstant, generate the declare block for the null constant.
3. ImplicitCastBuilder : replace NullExpression.INSTANCE with TypedNullConstant when necessary
Unit test: testICastNullExp.json.
isnull(unknownCol), isnotnull(unknowCol), 1 + unknowCol, 1.2 + unknowCol, nested with explicit cast function, etc.
Repository: drill-git
Description
-------
This patch is to provide implicit cast support in Drill.
The code change is mainly in two areas:
1. Function Resolver and related type cast rules : used to choose the best matched function implementation corresponding to a function name and its list of argument types. Type promotion would be implied, if there is no function implementation whose parameter types exactly match the argument types. Type promotion is implemented using the type precedence list. Type promotion is checked against a rule which will help implement a isCastable(from, to) method.
New files: 1) DefaultFunctionResolver.java
2) FunctionResolver
3) FunctionResolverFactor.java
4) OperatorFunctionResolver.java : reserved for future use.
5) ResolverTypePrecedence.java
6) TypeCastRules.java
7) UDFFunctionResolver.java : reserved for future use.
2. ImplicitCastBuilder : to inject implicit cast whenever necessary. After the function resolver returns a best matched function implementation, if the param type does not match the argument type, an implicit cast will be inserted on top of the argument.
- ImplicitCastBuilder uses a visitor to inject implicit cast.
- ImplicitCastBuilder will be part of ExpressionTreeMateralizer, after we materialize the expression tree.
- ExpressionValidator will be called after implicit cast is injected, in stead of after expression materialization.
- Because ImplicitCastBuilder requires a FunctionImplementationRegistry (to decide the best matched function implementation), and ImplicitCastBuilder is part of ExpressionTreeMaterializer, all the call of ExpressionTreeMaterializer is modified by adding a FunctionImplementationRegistry.
New files or modified files:
1)CastFunctionDefs.java
2)ExpressionTreeMaterializer.java
3)ImplicitCastBuilder.java
4)DrillFuncHolder.java : expose param type and size
5)FunctionImplementationRegistry.java : expose the list of function implementation.
6) other files because of the change to ExpressionTreeMaterializer.java.
Some other minor changes:
1. ArgumentValidator.ComparableArguments :
change allSame from true to false, since currently we have comparison operators defined over different types of inputs.
2. VectorUtil:
Add code to handle null value when display the result.
Diffs (updated)
-----
common/src/main/java/org/apache/drill/common/expression/ArgumentValidators.java cd30a06
common/src/main/java/org/apache/drill/common/expression/TypedNullConstant.java PRE-CREATION
common/src/main/java/org/apache/drill/common/expression/fn/CastFunctionDefs.java PRE-CREATION
exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java 5336c0e
exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java 587fe3c
exec/java-exec/src/main/java/org/apache/drill/exec/expr/ExpressionTreeMaterializer.java 5d576b1
exec/java-exec/src/main/java/org/apache/drill/exec/expr/ImplicitCastBuilder.java PRE-CREATION
exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFuncHolder.java e001ffe
exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionImplementationRegistry.java 5bbab76
exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/MathFunctions.java 288760b
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/AggBatch.java f652e35
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/filter/FilterRecordBatch.java fe298d6
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/MergeJoinBatch.java f3a32cd
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingRecordBatch.java fd392a3
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/orderedpartitioner/OrderedPartitionRecordBatch.java a3d1d09
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionSenderRootExec.java bc53bd2
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectRecordBatch.java 41a4c4d
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/sort/SortBatch.java 7881115
exec/java-exec/src/main/java/org/apache/drill/exec/resolver/DefaultFunctionResolver.java PRE-CREATION
exec/java-exec/src/main/java/org/apache/drill/exec/resolver/FunctionResolver.java PRE-CREATION
exec/java-exec/src/main/java/org/apache/drill/exec/resolver/FunctionResolverFactory.java PRE-CREATION
exec/java-exec/src/main/java/org/apache/drill/exec/resolver/OperatorFunctionResolver.java PRE-CREATION
exec/java-exec/src/main/java/org/apache/drill/exec/resolver/ResolverTypePrecedence.java PRE-CREATION
exec/java-exec/src/main/java/org/apache/drill/exec/resolver/TypeCastRules.java PRE-CREATION
exec/java-exec/src/main/java/org/apache/drill/exec/resolver/UDFFunctionResolver.java PRE-CREATION
exec/java-exec/src/main/java/org/apache/drill/exec/util/VectorUtil.java 8b23bcd
exec/java-exec/src/test/java/org/apache/drill/exec/expr/ExpressionTest.java 4bff5a3
exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestImplicitCastFunctions.java PRE-CREATION
exec/java-exec/src/test/java/org/apache/drill/exec/record/ExpressionTreeMaterializerTest.java 7ee1718
exec/java-exec/src/test/resources/functions/cast/testICastConstant.json PRE-CREATION
exec/java-exec/src/test/resources/functions/cast/testICastMockCol.json PRE-CREATION
exec/java-exec/src/test/resources/functions/cast/testICastNullExp.json PRE-CREATION
exec/ref/src/test/resources/simple_plan.json c0837e6
Diff: https://reviews.apache.org/r/16490/diff/
Testing
-------
testImplicitCastFunctions.java is used for unit test. It uses two physical plans
1. testICastConstant.json : the input expression is a literal constants. eg, 1 + 1.2, etc. Also, cover the nested function cases.
2. testICastMockCol.json : the input expression is a column from mock scan operator.
Thanks,
Jinfeng Ni