drill-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Mehant Baid" <baid.meh...@gmail.com>
Subject Re: Review Request 18376: Drill-379: support part of string and math functions
Date Mon, 03 Mar 2014 06:28:51 GMT

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


Only reviewed the Math Functions, still need to review string functions.


exec/java-exec/src/main/codegen/data/MathFunc.tdd
<https://reviews.apache.org/r/18376/#comment66743>

    Should we add another function "ceiling" (Simply an alias to ceil). Most Db's (sql-server,
oracle, postgres, mysql) support "ceiling" and some of them also have "ceil" as an alias.




exec/java-exec/src/main/codegen/data/MathFunc.tdd
<https://reviews.apache.org/r/18376/#comment66749>

    Is the round function with another argument specifying the number of digits to round to
in the second phase?



exec/java-exec/src/main/codegen/data/MathFunc.tdd
<https://reviews.apache.org/r/18376/#comment66745>

    Optiq seems to rewrite sqrt function with power function. Basically 
    
    sqrt(x) = power(x, -0.5)
    
    You think we should eliminate this?



exec/java-exec/src/main/codegen/data/MathFunc.tdd
<https://reviews.apache.org/r/18376/#comment66750>

    The output of sign is always going to be -1, 0, 1. So shouldn't the outputType be TinyInt?
    
    Even though Postgres seems to indicate that the return type is same as input, I tried
a couple of examples and even for floating point values the sign() function always returns
integers as opposed to 1.0 or -1.0? 


- Mehant Baid


On Feb. 23, 2014, 5:55 a.m., Jinfeng Ni wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18376/
> -----------------------------------------------------------
> 
> (Updated Feb. 23, 2014, 5:55 a.m.)
> 
> 
> Review request for drill and Mehant Baid.
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> 
> We add the support of some commonly used String functions and Math functions (over int,
floating-point number).
> 
> 1. like
> 2. similar
> 3. regexp_replace
> 4. char_length
> 4. oct_length
> 5. bit_length
> 6. position
> 7. strops
> 8. lower
> 9. upper
> 10. initcap
> 11. substring/substr
> 12. left
> 13. right
> 14. replace
> 15.lpad
> 16 rpad
> 17. ltrim
> 18. rtrim
> 19. concat.
> 
> Math : abs, ceil, floor, sqrt, sign, trunc.
> 
> 
> Diffs
> -----
> 
>   common/src/main/java/org/apache/drill/common/expression/OutputTypeDeterminer.java 66523c4

>   common/src/main/java/org/apache/drill/common/expression/fn/MathFunctions.java ee3a099

>   exec/java-exec/src/main/codegen/config.fmpp cd2b2cc 
>   exec/java-exec/src/main/codegen/data/MathFunc.tdd PRE-CREATION 
>   exec/java-exec/src/main/codegen/templates/MathFunctions.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/CharSubstring.java
f991a41 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/MathFunctions.java
ea251c3 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/RegexpUtil.java PRE-CREATION

>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/StringFunctionUtil.java
PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/StringFunctions.java
PRE-CREATION 
>   exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestSimpleFunctions.java
25daa7a 
>   exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestStringFunctions.java
PRE-CREATION 
>   exec/java-exec/src/test/resources/functions/string/testCharLength.json PRE-CREATION

>   exec/java-exec/src/test/resources/functions/string/testConcat.json PRE-CREATION 
>   exec/java-exec/src/test/resources/functions/string/testLeft.json PRE-CREATION 
>   exec/java-exec/src/test/resources/functions/string/testLike.json PRE-CREATION 
>   exec/java-exec/src/test/resources/functions/string/testLower.json PRE-CREATION 
>   exec/java-exec/src/test/resources/functions/string/testLpad.json PRE-CREATION 
>   exec/java-exec/src/test/resources/functions/string/testLtrim.json PRE-CREATION 
>   exec/java-exec/src/test/resources/functions/string/testPosition.json PRE-CREATION 
>   exec/java-exec/src/test/resources/functions/string/testRegexpReplace.json PRE-CREATION

>   exec/java-exec/src/test/resources/functions/string/testReplace.json PRE-CREATION 
>   exec/java-exec/src/test/resources/functions/string/testRight.json PRE-CREATION 
>   exec/java-exec/src/test/resources/functions/string/testRpad.json PRE-CREATION 
>   exec/java-exec/src/test/resources/functions/string/testRtrim.json PRE-CREATION 
>   exec/java-exec/src/test/resources/functions/string/testSimilar.json PRE-CREATION 
>   exec/java-exec/src/test/resources/functions/string/testSubstr.json PRE-CREATION 
>   exec/java-exec/src/test/resources/functions/string/testUpper.json PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/18376/diff/
> 
> 
> Testing
> -------
> 
> A JUnit test case is added, to test the string functions. 
> 
> 
> Thanks,
> 
> Jinfeng Ni
> 
>


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