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