> On Dec. 16, 2013, 10:01 p.m., Jacques Nadeau wrote: > > common/src/main/antlr3/org/apache/drill/common/expression/parser/ExprParser.g, > > line 99 > > <https://reviews.apache.org/r/15871/diff/4/?file=398248#file398248line99> > > > > As discussed, it is weird to treat the type as a logical expression. > > Can you please modify this to be a new struct that holds the information > > associated with charType and numType? Same for typeLen. Or maybe just > > build a more complicated parser rule? Otherwise it seems like a hack
Fix: use MajorType to hold all the information related to the target type (name, length, mode, etc). This also simplifies grammar rules. > On Dec. 16, 2013, 10:01 p.m., Jacques Nadeau wrote: > > common/src/main/java/org/apache/drill/common/expression/FunctionRegistry.java, > > line 44 > > <https://reviews.apache.org/r/15871/diff/4/?file=398251#file398251line44> > > > > Can you add these definitions so that they are automatically generated > > as part of type helper so we don't have to worry about maintenance in > > multiple places? fix: typeNameMap is no longed required in the code. The new code uses drill's internal type name as part of the cast function name. As such, no need for such typeNameMap structure any more. - Jinfeng ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15871/#review30518 ----------------------------------------------------------- On Dec. 16, 2013, 9:52 a.m., Jinfeng Ni wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/15871/ > ----------------------------------------------------------- > > (Updated Dec. 16, 2013, 9:52 a.m.) > > > Review request for drill. > > > Repository: drill-git > > > Description > ------- > > The explicit cast function is treated as a special function call. Its syntax > in logical/physical plan: > > cast (input_expr as int | bigint | float4 | float8 | varchar(length) | > varbinary(length) ) > > The main code changes : > 1) parser change to read expression in logical/physical plan, which may > contain an explicit cast function > 2) FunctionRegistry.java is added with two methods to build a logical > expression for cast function. > 3) Add 3 function template to generate the different type of cast functions : > > CastFunctionsSrcVarLen.java : when source type has varied length, target > type has fixed length. > CastFunctionsTargetVarLen.java: when source type has fixed length, target > type has varied length. > CastFunctionsSrcVarLenTargetVarLen.java: when both source and target type > have varied length. > > Currently, the following 6 types can be cast into each other > float4, float8, int, bigint, varchar, varbinary. > > > Diffs > ----- > > > common/src/main/antlr3/org/apache/drill/common/expression/parser/ExprLexer.g > 76da965 > > common/src/main/antlr3/org/apache/drill/common/expression/parser/ExprParser.g > 7fc1651 > common/src/main/java/org/apache/drill/common/expression/FunctionCall.java > a8b7e01 > > common/src/main/java/org/apache/drill/common/expression/FunctionDefinition.java > fb00dc7 > > common/src/main/java/org/apache/drill/common/expression/FunctionRegistry.java > 73e1925 > > common/src/main/java/org/apache/drill/common/expression/OutputTypeDeterminer.java > 2f583d5 > > common/src/main/java/org/apache/drill/common/expression/fn/CastFunctionDefs.java > PRE-CREATION > exec/java-exec/src/main/codegen/data/Casts.tdd 760b0f9 > exec/java-exec/src/main/codegen/templates/CastFunctions.java 85a00b0 > exec/java-exec/src/main/codegen/templates/CastFunctionsSrcVarLen.java > PRE-CREATION > > exec/java-exec/src/main/codegen/templates/CastFunctionsSrcVarLenTargetVarLen.java > PRE-CREATION > exec/java-exec/src/main/codegen/templates/CastFunctionsTargetVarLen.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/vector/ValueHolderHelper.java > 077c693 > exec/java-exec/src/main/java/org/apache/drill/exec/work/FragmentRunner.java > d003972 > > exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestCastFunctions.java > PRE-CREATION > exec/java-exec/src/test/resources/functions/cast/testCastBigInt.json > PRE-CREATION > exec/java-exec/src/test/resources/functions/cast/testCastFloat4.json > PRE-CREATION > exec/java-exec/src/test/resources/functions/cast/testCastFloat8.json > PRE-CREATION > exec/java-exec/src/test/resources/functions/cast/testCastInt.json > PRE-CREATION > exec/java-exec/src/test/resources/functions/cast/testCastNested.json > PRE-CREATION > exec/java-exec/src/test/resources/functions/cast/testCastVarBinary.json > PRE-CREATION > exec/java-exec/src/test/resources/functions/cast/testCastVarChar.json > PRE-CREATION > > Diff: https://reviews.apache.org/r/15871/diff/ > > > Testing > ------- > > 7 physical plans are used in junit testcase testCastFunctions.java > > 1) testCastInt.json : when target type is int, and src type is bigint, > float4, float8, varchar, varbinary. > 2) testCastBigInt.json: when target type is bigint, and src type is int, > float4, float8, varchar, varbinary. > 3) testCastFloat4.json: when target type is float4, and src type is int, > bigint, float8, varchar, varbinary. > 4) testCastFloat8.json: when target type is float8, and src type is int, > bigint, float4, varchar, varbinary > 5) testCastVarChar.json: when target tyep is varchar, and src type is int, > bigint, float4,float8, varbinary > 6) testCastVarBinary.json: when target type is varbinary, and src type is > int, bigint, float4, float8, varchar. > 7) testCastNested.json: test cast functions is nested in another cast > function, or another normal functions. > > > > Thanks, > > Jinfeng Ni > >
