> On Dec. 18, 2013, 5:07 p.m., Timothy Chen wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/work/FragmentRunner.java,
> >  line 96
> > <https://reviews.apache.org/r/15871/diff/4/?file=398262#file398262line96>
> >
> >     Was this intended to be here?

This is added when I debug code issue. For some reason, when fragmentrunner 
caught a failure, the stacktrace is not captured in the log sometimes. That's 
why I added this line of code. 

I removed this line of code. 


> On Dec. 18, 2013, 5:07 p.m., Timothy Chen wrote:
> > exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestCastFunctions.java,
> >  line 59
> > <https://reviews.apache.org/r/15871/diff/4/?file=398263#file398263line59>
> >
> >     Do you think we need to test cases when we cannot cast? And also since 
> > you set varchar(length) what happens if you set a smaller width than the 
> > actual content?

1. I added one test case to test when cast cause NumberFormatException, eg  
cast("abc" as int).

2. target type is varchar(length),
  2.1) when the input type is fixed-length type, and the varchar could not hold 
all the digits, there would be truncation. 

( testCastVarChar.json )
 cast(intcol as varchar(2)), when intcol = -214748364, the output is -2, 
                             when intcol = 2147483647, the output is 21.
 
  2.2) when the input type is var-length type as well
  if the input's length is longer than the target length, again, there would be 
truncation.
  cast("12345" as varchar(2)  ==> "12"
 
  if the input's length is smaller than the target length, then code just set 
the target length = input's length. 

    //if input length <= target_type length, do nothing
    //else truncate based on target_type length.     
    out.buffer = in.buffer;
    out.start =  in.start;
    if (in.end - in.start <= length.value) {
      out.end = in.end;
    } else {
      out.end = out.start + (int) length.value;
    }
  }

When truncation happens, one might want to issue a warning. Drill-319 is opened 
to address such issue. 


- Jinfeng


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15871/#review30662
-----------------------------------------------------------


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

Reply via email to