You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by Jinfeng Ni <jn...@maprtech.com> on 2014/02/21 22:29:44 UTC
Review Request 18376: Drill-379: support part of string and math functions
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18376/
-----------------------------------------------------------
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/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
Re: Review Request 18376: Drill-379: support part of string and math functions
Posted by Jacques Nadeau <ja...@apache.org>.
Similar to field injection using CDI, except that it will be part of run
time compilation rather than based on reflection. So basically something
similar to @Inject ByteBuf buffer
Then the execution engine manages the buffer creation.
On Tue, Mar 18, 2014 at 12:05 AM, Timothy Chen <tn...@gmail.com> wrote:
> Mind clarifying what does psuedo injection means?
>
> Thanks,
>
> Tim
>
> > On Mar 17, 2014, at 10:18 PM, "Jacques Nadeau" <ja...@gmail.com>
> wrote:
> >
> >
> > -----------------------------------------------------------
> > This is an automatically generated e-mail. To reply, visit:
> > https://reviews.apache.org/r/18376/#review37537
> > -----------------------------------------------------------
> >
> > Ship it!
> >
> >
> > Looks good. One thing I'd like to address as a separate issue...
> Jinfeng, can you please open a new JIRA to move away from using wrapped
> buffers. This is a dangerous approach as these buffers are big endian
> versus Drill's native format of little endian. Additionally, this requires
> us to pull data on heap to execute these functions. We should move to an
> approach where anyone who wants a buffer uses pseudo-injection to get
> access to buffers and the context is responsible for cleaning things up.
> >
> > - Jacques Nadeau
> >
> >
> >> On March 11, 2014, 11:59 p.m., Jinfeng Ni wrote:
> >>
> >> -----------------------------------------------------------
> >> This is an automatically generated e-mail. To reply, visit:
> >> https://reviews.apache.org/r/18376/
> >> -----------------------------------------------------------
> >>
> >> (Updated March 11, 2014, 11:59 p.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
> 69acf12
> >>
> common/src/main/java/org/apache/drill/common/expression/fn/MathFunctions.java
> ee3a099
> >> exec/java-exec/src/main/codegen/config.fmpp 8f1060a
> >> 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
> f4e2060
> >>
> 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
> f6a8096
> >>
> 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
> >
>
Re: Review Request 18376: Drill-379: support part of string and math functions
Posted by Timothy Chen <tn...@gmail.com>.
Mind clarifying what does psuedo injection means?
Thanks,
Tim
> On Mar 17, 2014, at 10:18 PM, "Jacques Nadeau" <ja...@gmail.com> wrote:
>
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18376/#review37537
> -----------------------------------------------------------
>
> Ship it!
>
>
> Looks good. One thing I'd like to address as a separate issue... Jinfeng, can you please open a new JIRA to move away from using wrapped buffers. This is a dangerous approach as these buffers are big endian versus Drill's native format of little endian. Additionally, this requires us to pull data on heap to execute these functions. We should move to an approach where anyone who wants a buffer uses pseudo-injection to get access to buffers and the context is responsible for cleaning things up.
>
> - Jacques Nadeau
>
>
>> On March 11, 2014, 11:59 p.m., Jinfeng Ni wrote:
>>
>> -----------------------------------------------------------
>> This is an automatically generated e-mail. To reply, visit:
>> https://reviews.apache.org/r/18376/
>> -----------------------------------------------------------
>>
>> (Updated March 11, 2014, 11:59 p.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 69acf12
>> common/src/main/java/org/apache/drill/common/expression/fn/MathFunctions.java ee3a099
>> exec/java-exec/src/main/codegen/config.fmpp 8f1060a
>> 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 f4e2060
>> 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 f6a8096
>> 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
>
Re: Review Request 18376: Drill-379: support part of string and math
functions
Posted by Jacques Nadeau <ja...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18376/#review37537
-----------------------------------------------------------
Ship it!
Looks good. One thing I'd like to address as a separate issue... Jinfeng, can you please open a new JIRA to move away from using wrapped buffers. This is a dangerous approach as these buffers are big endian versus Drill's native format of little endian. Additionally, this requires us to pull data on heap to execute these functions. We should move to an approach where anyone who wants a buffer uses pseudo-injection to get access to buffers and the context is responsible for cleaning things up.
- Jacques Nadeau
On March 11, 2014, 11:59 p.m., Jinfeng Ni wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18376/
> -----------------------------------------------------------
>
> (Updated March 11, 2014, 11:59 p.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 69acf12
> common/src/main/java/org/apache/drill/common/expression/fn/MathFunctions.java ee3a099
> exec/java-exec/src/main/codegen/config.fmpp 8f1060a
> 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 f4e2060
> 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 f6a8096
> 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
>
>
Re: Review Request 18376: Drill-379: support part of string and math
functions
Posted by Mehant Baid <ba...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18376/#review37181
-----------------------------------------------------------
Looks good overall. Few more minor comments in addition to the previous ones.
One generic comment is to test out these functions using SQL as well, just to make sure the Optiq to Drill expression transformations is being done correctly.
common/src/main/java/org/apache/drill/common/expression/OutputTypeDeterminer.java
<https://reviews.apache.org/r/18376/#comment68597>
You can reset this file since it doesn't have any changes.
exec/java-exec/src/main/codegen/data/MathFunc.tdd
<https://reviews.apache.org/r/18376/#comment68599>
The format of this tdd file is very similar to MathFunctionTypes.tdd. Do you think it makes sense to consolidate all the definitions in a single tdd file? It might be easier to keep track of what has been implemented
exec/java-exec/src/main/codegen/data/MathFunc.tdd
<https://reviews.apache.org/r/18376/#comment68598>
Divide is already included in MathFunctionTypes.tdd, you can get rid of it.
exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/StringFunctionUtil.java
<https://reviews.apache.org/r/18376/#comment68778>
I think 0x128 is incorrect? Should be in decimal.
exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/StringFunctionUtil.java
<https://reviews.apache.org/r/18376/#comment68779>
Same as above should be in decimal
exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/StringFunctionUtil.java
<https://reviews.apache.org/r/18376/#comment68780>
Is there a reason we are inspecting every byte? For every character we know the length by examining the first byte, so can't we skip over the remaining bytes?
exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/StringFunctions.java
<https://reviews.apache.org/r/18376/#comment68793>
You aren't using the workspace variable, can you get rid of it?
Could you remove it from the other functions that don't use it as well?
exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestStringFunctions.java
<https://reviews.apache.org/r/18376/#comment68794>
Do you think its possible to add a couple of tests with non-ascii characters so we can hit the multi byte code path?
- Mehant Baid
On March 11, 2014, 11:59 p.m., Jinfeng Ni wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18376/
> -----------------------------------------------------------
>
> (Updated March 11, 2014, 11:59 p.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 69acf12
> common/src/main/java/org/apache/drill/common/expression/fn/MathFunctions.java ee3a099
> exec/java-exec/src/main/codegen/config.fmpp 8f1060a
> 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 f4e2060
> 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 f6a8096
> 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
>
>
Re: Review Request 18376: Drill-379: support part of string and math
functions
Posted by Jinfeng Ni <jn...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18376/
-----------------------------------------------------------
(Updated March 11, 2014, 4:59 p.m.)
Review request for drill and Mehant Baid.
Changes
-------
Rebase on latest master branch.
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 (updated)
-----
common/src/main/java/org/apache/drill/common/expression/OutputTypeDeterminer.java 69acf12
common/src/main/java/org/apache/drill/common/expression/fn/MathFunctions.java ee3a099
exec/java-exec/src/main/codegen/config.fmpp 8f1060a
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 f4e2060
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 f6a8096
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
Re: Review Request 18376: Drill-379: support part of string and math
functions
Posted by Mehant Baid <ba...@gmail.com>.
-----------------------------------------------------------
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
>
>
Re: Review Request 18376: Drill-379: support part of string and math
functions
Posted by Jinfeng Ni <jn...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18376/
-----------------------------------------------------------
(Updated Feb. 22, 2014, 9:55 p.m.)
Review request for drill and Mehant Baid.
Changes
-------
Add 'like' and 'similar' function, as well as the junit test case.
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 (updated)
-----
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