You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by Mehant Baid <ba...@gmail.com> on 2013/12/13 01:56:30 UTC
Review Request 16230: DRILL-320: Rework math functions to use codegen.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16230/
-----------------------------------------------------------
Review request for drill and Jacques Nadeau.
Bugs: DRILL-320
https://issues.apache.org/jira/browse/DRILL-320
Repository: drill-git
Description
-------
Rework the implementations of existing math functions to use codegen so that we have a more complete list of implementations. Previous implementations were done manually so we only had a handful of implementations for each function. Now that we are using freemarker to generate code during compile time we can easily have all the required implementations.
We were also missing any implementation for multiply and divide functions, so added that as part of this JIRA.
The implementations added here are for inputs with similar types (eg: for the function 'add', the inputs can be of type INT, INT or FLOAT, FLOAT etc). The expectation is that if we have implementations for similar types of input, once we have implicit casting we should be able to cast one of the inputs so that both the inputs are of the same type.
MathFunctionTypes.tdd, MathFunctionTemplates.java:
The above two files are used by freemarker to generate code for all the types specified in MathFunctionTypes.tdd using the template specified in MathFunctionTemplates.java.
ComparisonFunctions.java, SimpleRepeatedFunctions.java:
Simple modification in these files to use the correct class for logging purposes.
MathFunctions.java:
This class used to contain some implementations for add, subtract methods. Removed those from here, since we have all the implementations via codegen.
TestMathFunctions.java, simple_math_functions.json:
Added really basic test cases for add and multiply functions.
Diffs
-----
exec/java-exec/src/main/codegen/config.fmpp cd2b2cc
exec/java-exec/src/main/codegen/data/MathFunctionTypes.tdd PRE-CREATION
exec/java-exec/src/main/codegen/templates/MathFunctionTemplates.java PRE-CREATION
exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/ComparisonFunctions.java 15034cd
exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/MathFunctions.java 288760b
exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/SimpleRepeatedFunctions.java 2f838b1
exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/TestMathFunctions.java PRE-CREATION
exec/java-exec/src/test/resources/functions/simple_math_functions.json PRE-CREATION
Diff: https://reviews.apache.org/r/16230/diff/
Testing
-------
Manual testing using sqlline.
Added basic test cases in TestMathFunctions.java, simple addition and multiplication of constants.
Thanks,
Mehant Baid
Re: Review Request 16230: DRILL-320: Rework math functions to use codegen.
Posted by Jinfeng Ni <jn...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16230/#review31507
-----------------------------------------------------------
exec/java-exec/src/main/codegen/templates/MathFunctionTemplates.java
<https://reviews.apache.org/r/16230/#comment60008>
It would be nice if you can adjust the format. Currently, in the generated code, setup() and eval() look like at the same indent level as the class, in stead of within the class.
Similar suggestion regarding the closing "}" at line 75 and 77.
exec/java-exec/src/main/codegen/templates/MathFunctionTemplates.java
<https://reviews.apache.org/r/16230/#comment60006>
If you add a new field (javaOp) to inputType, you may not need use #if #elseif #elseif ...
In stead the whole #if block could be simplified with :
out.value = (${type.castType}) (in1.value ${inputType.javaOp} in2.value);
exec/java-exec/src/main/codegen/templates/MathFunctionTemplates.java
<https://reviews.apache.org/r/16230/#comment60007>
"substract" need use "-" operator.
- Jinfeng Ni
On Dec. 18, 2013, 10:43 p.m., Mehant Baid wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16230/
> -----------------------------------------------------------
>
> (Updated Dec. 18, 2013, 10:43 p.m.)
>
>
> Review request for drill and Jacques Nadeau.
>
>
> Bugs: DRILL-320
> https://issues.apache.org/jira/browse/DRILL-320
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> Rework the implementations of existing math functions to use codegen so that we have a more complete list of implementations. Previous implementations were done manually so we only had a handful of implementations for each function. Now that we are using freemarker to generate code during compile time we can easily have all the required implementations.
>
> We were also missing any implementation for multiply and divide functions, so added that as part of this JIRA.
>
> The implementations added here are for inputs with similar types (eg: for the function 'add', the inputs can be of type INT, INT or FLOAT, FLOAT etc). The expectation is that if we have implementations for similar types of input, once we have implicit casting we should be able to cast one of the inputs so that both the inputs are of the same type.
>
>
> MathFunctionTypes.tdd, MathFunctionTemplates.java:
> The above two files are used by freemarker to generate code for all the types specified in MathFunctionTypes.tdd using the template specified in MathFunctionTemplates.java.
>
> ComparisonFunctions.java, SimpleRepeatedFunctions.java:
> Simple modification in these files to use the correct class for logging purposes.
>
> MathFunctions.java:
> This class used to contain some implementations for add, subtract methods. Removed those from here, since we have all the implementations via codegen.
>
> TestMathFunctions.java, simple_math_functions.json:
> Added really basic test cases for add and multiply functions.
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/codegen/config.fmpp cd2b2cc
> exec/java-exec/src/main/codegen/data/MathFunctionTypes.tdd PRE-CREATION
> exec/java-exec/src/main/codegen/templates/MathFunctionTemplates.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/ComparisonFunctions.java 15034cd
> exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/MathFunctions.java 288760b
> exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/SimpleRepeatedFunctions.java 2f838b1
> exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/TestMathFunctions.java PRE-CREATION
> exec/java-exec/src/test/resources/functions/simple_math_functions.json PRE-CREATION
>
> Diff: https://reviews.apache.org/r/16230/diff/
>
>
> Testing
> -------
>
> Manual testing using sqlline.
>
> Added basic test cases in TestMathFunctions.java, simple addition and multiplication of constants.
>
>
> Thanks,
>
> Mehant Baid
>
>
Re: Review Request 16230: DRILL-320: Rework math functions to use codegen.
Posted by Jason Altekruse <al...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16230/#review34016
-----------------------------------------------------------
exec/java-exec/src/main/codegen/data/MathFunctionTypes.tdd
<https://reviews.apache.org/r/16230/#comment63948>
These inner lists are currently all identical (it looks that way, but I actually had to use diff to make sure), is there any chance we would want to declare future binary functions with different casting rules? Would it make sense to declare this list once outside of all of the json objects for the function descriptions? If we need to have a mapping between functions and these longish lists of casting rules we could include a list of function names to apply them to, but for now it does not seem like that is needed.
- Jason Altekruse
On Feb. 8, 2014, 7:03 a.m., Mehant Baid wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16230/
> -----------------------------------------------------------
>
> (Updated Feb. 8, 2014, 7:03 a.m.)
>
>
> Review request for drill and Jacques Nadeau.
>
>
> Bugs: DRILL-320
> https://issues.apache.org/jira/browse/DRILL-320
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> Rework the implementations of existing math functions to use codegen so that we have a more complete list of implementations. Previous implementations were done manually so we only had a handful of implementations for each function. Now that we are using freemarker to generate code during compile time we can easily have all the required implementations.
>
> We were also missing any implementation for multiply and divide functions, so added that as part of this JIRA.
>
> The implementations added here are for inputs with similar types (eg: for the function 'add', the inputs can be of type INT, INT or FLOAT, FLOAT etc). The expectation is that if we have implementations for similar types of input, once we have implicit casting we should be able to cast one of the inputs so that both the inputs are of the same type.
>
>
> MathFunctionTypes.tdd, MathFunctionTemplates.java:
> The above two files are used by freemarker to generate code for all the types specified in MathFunctionTypes.tdd using the template specified in MathFunctionTemplates.java.
>
> ComparisonFunctions.java, SimpleRepeatedFunctions.java:
> Simple modification in these files to use the correct class for logging purposes.
>
> MathFunctions.java:
> This class used to contain some implementations for add, subtract methods. Removed those from here, since we have all the implementations via codegen.
>
> TestMathFunctions.java, simple_math_functions.json:
> Added really basic test cases for add and multiply functions.
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/codegen/config.fmpp cd2b2cc
> exec/java-exec/src/main/codegen/data/MathFunctionTypes.tdd PRE-CREATION
> exec/java-exec/src/main/codegen/templates/MathFunctionTemplates.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/MathFunctions.java ea251c3
> exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/TestMathFunctions.java PRE-CREATION
> exec/java-exec/src/test/resources/functions/simple_math_functions.json PRE-CREATION
>
> Diff: https://reviews.apache.org/r/16230/diff/
>
>
> Testing
> -------
>
> Manual testing using sqlline.
>
> Added basic test cases in TestMathFunctions.java, simple addition and multiplication of constants.
>
>
> Thanks,
>
> Mehant Baid
>
>
Re: Review Request 16230: DRILL-320: Rework math functions to use codegen.
Posted by Mehant Baid <ba...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16230/
-----------------------------------------------------------
(Updated Feb. 8, 2014, 7:03 a.m.)
Review request for drill and Jacques Nadeau.
Changes
-------
Revised patch so it applies cleanly on top of Master.
Bugs: DRILL-320
https://issues.apache.org/jira/browse/DRILL-320
Repository: drill-git
Description
-------
Rework the implementations of existing math functions to use codegen so that we have a more complete list of implementations. Previous implementations were done manually so we only had a handful of implementations for each function. Now that we are using freemarker to generate code during compile time we can easily have all the required implementations.
We were also missing any implementation for multiply and divide functions, so added that as part of this JIRA.
The implementations added here are for inputs with similar types (eg: for the function 'add', the inputs can be of type INT, INT or FLOAT, FLOAT etc). The expectation is that if we have implementations for similar types of input, once we have implicit casting we should be able to cast one of the inputs so that both the inputs are of the same type.
MathFunctionTypes.tdd, MathFunctionTemplates.java:
The above two files are used by freemarker to generate code for all the types specified in MathFunctionTypes.tdd using the template specified in MathFunctionTemplates.java.
ComparisonFunctions.java, SimpleRepeatedFunctions.java:
Simple modification in these files to use the correct class for logging purposes.
MathFunctions.java:
This class used to contain some implementations for add, subtract methods. Removed those from here, since we have all the implementations via codegen.
TestMathFunctions.java, simple_math_functions.json:
Added really basic test cases for add and multiply functions.
Diffs (updated)
-----
exec/java-exec/src/main/codegen/config.fmpp cd2b2cc
exec/java-exec/src/main/codegen/data/MathFunctionTypes.tdd PRE-CREATION
exec/java-exec/src/main/codegen/templates/MathFunctionTemplates.java PRE-CREATION
exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/MathFunctions.java ea251c3
exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/TestMathFunctions.java PRE-CREATION
exec/java-exec/src/test/resources/functions/simple_math_functions.json PRE-CREATION
Diff: https://reviews.apache.org/r/16230/diff/
Testing
-------
Manual testing using sqlline.
Added basic test cases in TestMathFunctions.java, simple addition and multiplication of constants.
Thanks,
Mehant Baid
Re: Review Request 16230: DRILL-320: Rework math functions to use codegen.
Posted by Mehant Baid <ba...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16230/
-----------------------------------------------------------
(Updated Jan. 10, 2014, 6:34 a.m.)
Review request for drill and Jacques Nadeau.
Changes
-------
Incorporated Jinfeng's suggestions.
Bugs: DRILL-320
https://issues.apache.org/jira/browse/DRILL-320
Repository: drill-git
Description
-------
Rework the implementations of existing math functions to use codegen so that we have a more complete list of implementations. Previous implementations were done manually so we only had a handful of implementations for each function. Now that we are using freemarker to generate code during compile time we can easily have all the required implementations.
We were also missing any implementation for multiply and divide functions, so added that as part of this JIRA.
The implementations added here are for inputs with similar types (eg: for the function 'add', the inputs can be of type INT, INT or FLOAT, FLOAT etc). The expectation is that if we have implementations for similar types of input, once we have implicit casting we should be able to cast one of the inputs so that both the inputs are of the same type.
MathFunctionTypes.tdd, MathFunctionTemplates.java:
The above two files are used by freemarker to generate code for all the types specified in MathFunctionTypes.tdd using the template specified in MathFunctionTemplates.java.
ComparisonFunctions.java, SimpleRepeatedFunctions.java:
Simple modification in these files to use the correct class for logging purposes.
MathFunctions.java:
This class used to contain some implementations for add, subtract methods. Removed those from here, since we have all the implementations via codegen.
TestMathFunctions.java, simple_math_functions.json:
Added really basic test cases for add and multiply functions.
Diffs (updated)
-----
exec/java-exec/src/main/codegen/config.fmpp cd2b2cc
exec/java-exec/src/main/codegen/data/MathFunctionTypes.tdd PRE-CREATION
exec/java-exec/src/main/codegen/templates/MathFunctionTemplates.java PRE-CREATION
exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/ComparisonFunctions.java 15034cd
exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/MathFunctions.java 288760b
exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/SimpleRepeatedFunctions.java 2f838b1
exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/TestMathFunctions.java PRE-CREATION
exec/java-exec/src/test/resources/functions/simple_math_functions.json PRE-CREATION
Diff: https://reviews.apache.org/r/16230/diff/
Testing
-------
Manual testing using sqlline.
Added basic test cases in TestMathFunctions.java, simple addition and multiplication of constants.
Thanks,
Mehant Baid
Re: Review Request 16230: DRILL-320: Rework math functions to use codegen.
Posted by Mehant Baid <ba...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16230/
-----------------------------------------------------------
(Updated Dec. 19, 2013, 6:43 a.m.)
Review request for drill and Jacques Nadeau.
Changes
-------
Changed the rules of math functions so we model after Postgres. The output type would match the input type (the larger one in case of multiple inputs). For example if the input types to an add function are two tinyint's and we overflow then we simply lose the precision. We manually don't use a bigger holder to store the result of the addition.
Bugs: DRILL-320
https://issues.apache.org/jira/browse/DRILL-320
Repository: drill-git
Description
-------
Rework the implementations of existing math functions to use codegen so that we have a more complete list of implementations. Previous implementations were done manually so we only had a handful of implementations for each function. Now that we are using freemarker to generate code during compile time we can easily have all the required implementations.
We were also missing any implementation for multiply and divide functions, so added that as part of this JIRA.
The implementations added here are for inputs with similar types (eg: for the function 'add', the inputs can be of type INT, INT or FLOAT, FLOAT etc). The expectation is that if we have implementations for similar types of input, once we have implicit casting we should be able to cast one of the inputs so that both the inputs are of the same type.
MathFunctionTypes.tdd, MathFunctionTemplates.java:
The above two files are used by freemarker to generate code for all the types specified in MathFunctionTypes.tdd using the template specified in MathFunctionTemplates.java.
ComparisonFunctions.java, SimpleRepeatedFunctions.java:
Simple modification in these files to use the correct class for logging purposes.
MathFunctions.java:
This class used to contain some implementations for add, subtract methods. Removed those from here, since we have all the implementations via codegen.
TestMathFunctions.java, simple_math_functions.json:
Added really basic test cases for add and multiply functions.
Diffs (updated)
-----
exec/java-exec/src/main/codegen/config.fmpp cd2b2cc
exec/java-exec/src/main/codegen/data/MathFunctionTypes.tdd PRE-CREATION
exec/java-exec/src/main/codegen/templates/MathFunctionTemplates.java PRE-CREATION
exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/ComparisonFunctions.java 15034cd
exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/MathFunctions.java 288760b
exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/SimpleRepeatedFunctions.java 2f838b1
exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/TestMathFunctions.java PRE-CREATION
exec/java-exec/src/test/resources/functions/simple_math_functions.json PRE-CREATION
Diff: https://reviews.apache.org/r/16230/diff/
Testing
-------
Manual testing using sqlline.
Added basic test cases in TestMathFunctions.java, simple addition and multiplication of constants.
Thanks,
Mehant Baid
Re: Review Request 16230: DRILL-320: Rework math functions to use codegen.
Posted by Jacques Nadeau <ja...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16230/#review30520
-----------------------------------------------------------
exec/java-exec/src/main/codegen/data/MathFunctionTypes.tdd
<https://reviews.apache.org/r/16230/#comment58453>
Where did you get these conversion rules? For example, TinyInt + TinyInt => BigInt?
- Jacques Nadeau
On Dec. 13, 2013, 12:56 a.m., Mehant Baid wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16230/
> -----------------------------------------------------------
>
> (Updated Dec. 13, 2013, 12:56 a.m.)
>
>
> Review request for drill and Jacques Nadeau.
>
>
> Bugs: DRILL-320
> https://issues.apache.org/jira/browse/DRILL-320
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> Rework the implementations of existing math functions to use codegen so that we have a more complete list of implementations. Previous implementations were done manually so we only had a handful of implementations for each function. Now that we are using freemarker to generate code during compile time we can easily have all the required implementations.
>
> We were also missing any implementation for multiply and divide functions, so added that as part of this JIRA.
>
> The implementations added here are for inputs with similar types (eg: for the function 'add', the inputs can be of type INT, INT or FLOAT, FLOAT etc). The expectation is that if we have implementations for similar types of input, once we have implicit casting we should be able to cast one of the inputs so that both the inputs are of the same type.
>
>
> MathFunctionTypes.tdd, MathFunctionTemplates.java:
> The above two files are used by freemarker to generate code for all the types specified in MathFunctionTypes.tdd using the template specified in MathFunctionTemplates.java.
>
> ComparisonFunctions.java, SimpleRepeatedFunctions.java:
> Simple modification in these files to use the correct class for logging purposes.
>
> MathFunctions.java:
> This class used to contain some implementations for add, subtract methods. Removed those from here, since we have all the implementations via codegen.
>
> TestMathFunctions.java, simple_math_functions.json:
> Added really basic test cases for add and multiply functions.
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/codegen/config.fmpp cd2b2cc
> exec/java-exec/src/main/codegen/data/MathFunctionTypes.tdd PRE-CREATION
> exec/java-exec/src/main/codegen/templates/MathFunctionTemplates.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/ComparisonFunctions.java 15034cd
> exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/MathFunctions.java 288760b
> exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/SimpleRepeatedFunctions.java 2f838b1
> exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/TestMathFunctions.java PRE-CREATION
> exec/java-exec/src/test/resources/functions/simple_math_functions.json PRE-CREATION
>
> Diff: https://reviews.apache.org/r/16230/diff/
>
>
> Testing
> -------
>
> Manual testing using sqlline.
>
> Added basic test cases in TestMathFunctions.java, simple addition and multiplication of constants.
>
>
> Thanks,
>
> Mehant Baid
>
>