----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/17881/#review34680 -----------------------------------------------------------
Ship it! Changes look good to me. One clarification: your comments in the description mention changes to the DefaultFunctionResolver but I did not see that in the diffs. Did you mean the cost adjustment done in the TypeCastRules ? The code change there looks reasonable to me. - Aman Sinha On Feb. 9, 2014, 11:48 p.m., Jason Altekruse wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/17881/ > ----------------------------------------------------------- > > (Updated Feb. 9, 2014, 11:48 p.m.) > > > Review request for drill. > > > Repository: drill-git > > > Description > ------- > > Added custom null handling for hash functions, to override the privies > NULL_IF_NULL handling. Added Integer and Float literals to the logical > expression package and corresponding methods in the various visitor > implementations. Also added new Hash functions for double and float values > based on the doubleToLongBits and floatToIntBits methods in the corresponding > boxed primitive classes together with the previously used murmur3_128 hash > algorithm used for plain Ints and Longs. > > To enable the correct function resolution the cost calculation in the > DefaultFunctionResolver was modified slightly. It now gives a higher cost to > functions that take a nullable value, and have internal null handling, when a > non-nullable is provided as an argument. As these functions will have to make > a null check on each value, we want to avoid the null check and choose an > implementation of the function designed specifically for non-null values if > available. > > > Diffs > ----- > > > common/src/main/java/org/apache/drill/common/expression/ExpressionValidator.java > 11d97e8 > > common/src/main/java/org/apache/drill/common/expression/ValueExpressions.java > 9137356 > > common/src/main/java/org/apache/drill/common/expression/visitors/AbstractExprVisitor.java > 711bdb8 > > common/src/main/java/org/apache/drill/common/expression/visitors/AggregateChecker.java > 128f284 > > common/src/main/java/org/apache/drill/common/expression/visitors/ConstantChecker.java > 02ce231 > > common/src/main/java/org/apache/drill/common/expression/visitors/ExprVisitor.java > db34ab7 > > common/src/main/java/org/apache/drill/common/expression/visitors/SimpleExprVisitor.java > 65a939b > > exec/java-exec/src/main/java/org/apache/drill/exec/compile/sig/ConstantExpressionIdentifier.java > 46dbbe9 > > exec/java-exec/src/main/java/org/apache/drill/exec/expr/ExpressionTreeMaterializer.java > 36433ad > > exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/HashFunctions.java > 76683f3 > exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/IsNull.java > a042a5a > > exec/java-exec/src/main/java/org/apache/drill/exec/resolver/ResolverTypePrecedence.java > 26ac961 > > exec/java-exec/src/main/java/org/apache/drill/exec/resolver/TypeCastRules.java > b16d1ff > > exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestSimpleFunctions.java > 25daa7a > > exec/ref/src/main/java/org/apache/drill/exec/ref/eval/SimpleEvaluationVisitor.java > 1b04880 > > Diff: https://reviews.apache.org/r/17881/diff/ > > > Testing > ------- > > > Thanks, > > Jason Altekruse > >
