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