----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27711/#review71600 -----------------------------------------------------------
Seems at least one unit testcase in excc/java_exec failed with different results from expected results. Failed tests: TestJoinNullable.testMergeInnerJoinOnNullableColumns:60 Received unexepcted number of rows in output: expected=1, received=0 expected:<1> but was:<0> exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionGenerationHelper.java <https://reviews.apache.org/r/27711/#comment117417> Is the code deleted when resolving merging conflicts? I think line 46 - 48 are needed for throwing clearer error message, when the query tries to order by , group by, or compare a complex type field. exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/ComparisonFunctions.java <https://reviews.apache.org/r/27711/#comment117418> I think it's fine to delete this class, since the code has been moved to freemarker template. exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/TopN/TopNBatch.java <https://reviews.apache.org/r/27711/#comment117424> Can you refactor this logic into a common method? I saw several places need use this logic to get null_high. Also, it would be nice to add comment why the logical expression would cover different combination of Direction / NullDirection, including UNSPECIFIED. - Jinfeng Ni On Feb. 6, 2015, 3:22 p.m., Daniel Barclay wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/27711/ > ----------------------------------------------------------- > > (Updated Feb. 6, 2015, 3:22 p.m.) > > > Review request for drill, Jinfeng Ni and Mehant Baid. > > > Repository: drill-git > > > Description > ------- > > Main change: Augmented *ordering* comparison function templates > and calls to to order NULL values correctly (per "NULLS FIRST", > "NULLS LAST", or correct default (depending on whether ordering is > ascending or descending) > * Cloned each "compare_to" function template into > "compare_to_nulls_high" and "compare_to_nulls_low" versions > and adjusted to handle NULL correctly. > * Added corresponding new version of getComparator(...). > * Updated code around calls to getComparator(...) re NULL ordering. > * Added test class and test data files. > > > Diffs > ----- > > .gitignore 838ea6b > common/src/main/java/org/apache/drill/common/logical/data/Order.java > dada606 > exec/java-exec/src/main/codegen/data/CompareTypes.tdd f384d52 > exec/java-exec/src/main/codegen/templates/ComparisonFunctions.java 628277c > exec/java-exec/src/main/codegen/templates/DateIntervalFunctions.java > 8fe13bb > exec/java-exec/src/main/codegen/templates/Decimal/CastVarCharDecimal.java > 960368a > exec/java-exec/src/main/codegen/templates/Decimal/DecimalFunctions.java > 0c4af01 > > exec/java-exec/src/main/java/org/apache/drill/exec/expr/annotations/FunctionTemplate.java > 1f732a3 > > exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionGenerationHelper.java > d007d7c > > exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/ComparisonFunctions.java > bf42ce6 > > exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/ComparisonFunctionsNullable.java > 570aaeb > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/TopN/TopNBatch.java > 9829fc6 > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/StreamingAggBatch.java > 860627d > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/MergeJoinBatch.java > 257b93e > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingRecordBatch.java > d78ba8e > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/orderedpartitioner/OrderedPartitionRecordBatch.java > a062074 > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/sort/SortBatch.java > 19f5423 > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/StreamingWindowFrameRecordBatch.java > 26d23f2 > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/ExternalSortBatch.java > 9026661 > > exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/QueryResultHandler.java > 9015a16 > > exec/jdbc/src/test/java/org/apache/drill/jdbc/test/JdbcNullOrderingAndGroupingTest.java > PRE-CREATION > exec/jdbc/src/test/java/org/apache/drill/jdbc/test/TestJdbcQuery.java > b627c38 > exec/jdbc/src/test/resources/donuts.json PRE-CREATION > exec/jdbc/src/test/resources/null_ordering_and_grouping_data.json > PRE-CREATION > pom.xml 17f0e09 > > Diff: https://reviews.apache.org/r/27711/diff/ > > > Testing > ------- > > Ran new fix-specific unit tests. > > > Thanks, > > Daniel Barclay > >
