drill-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jinfeng Ni" <...@maprtech.com>
Subject Re: Review Request 15871: DRILL-316. Explicit cast support in drill execution engine .
Date Thu, 19 Dec 2013 02:03:11 GMT


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


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message