> 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 > >
