----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31707/#review75407 -----------------------------------------------------------
Sending some initial comments. exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/union/UnionAllRecordBatch.java <https://reviews.apache.org/r/31707/#comment122444> This should ideally be just UnionAll. Is an import missing ? exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/union/UnionAllRecordBatch.java <https://reviews.apache.org/r/31707/#comment122737> Won't this convert to nullable even if the desired output type is non-nullable ? Also, I think in general most of this logic for deciding the casting should live outside of the UnionAll code in some common utility class such that it is used by multiple operators. exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/union/UnionAllRecordBatch.java <https://reviews.apache.org/r/31707/#comment122743> Can you run some tests with TPCH SF1 such that we are testing with larger numbers of record batches ? Also test a case where left side has 0 rows, right side has non-zero. exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/union/UnionAllRecordBatch.java <https://reviews.apache.org/r/31707/#comment122729> You should identify whether the schema change occurred on left or right input. Pls change the error to a clearer one, e.g "Schema change detected in <left/right> input of Union-All. This is not currently supported." exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/PreProcessLogicalRel.java <https://reviews.apache.org/r/31707/#comment122538> Error message should be clearer: "Union-all over schema-less tables must specify the columns explicitly" exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/visitor/FinalColumnReorderer.java <https://reviews.apache.org/r/31707/#comment122728> Is this going to force a Project on the right side even if the column names, ordinals and number of columns are the same as the ones on the left side ? I think we should avoid passing a 'force' flag here. - Aman Sinha On March 5, 2015, 12:27 a.m., Sean Hsuan-Yi Chu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/31707/ > ----------------------------------------------------------- > > (Updated March 5, 2015, 12:27 a.m.) > > > Review request for drill, Aman Sinha and Jinfeng Ni. > > > Bugs: DRILL-2207 > https://issues.apache.org/jira/browse/DRILL-2207 > > > Repository: drill-git > > > Description > ------- > > DRILL-2207: New Union-All Implementation > > > Diffs > ----- > > > exec/java-exec/src/main/java/org/apache/drill/exec/expr/ExpressionTreeMaterializer.java > 3565bf4 > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/union/UnionAll.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/union/UnionAllRecordBatch.java > 99aec92 > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/union/UnionAllTemplate.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/common/DrillRelOptUtil.java > 270462b > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/PreProcessLogicalRel.java > 4c9d301 > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/visitor/FinalColumnReorderer.java > 60a9e4b > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/UnsupportedOperatorsVisitor.java > dcd5ebf > > exec/java-exec/src/main/java/org/apache/drill/exec/record/VectorContainer.java > baf74b1 > > exec/java-exec/src/main/java/org/apache/drill/exec/resolver/TypeCastRules.java > f5b0de4 > exec/java-exec/src/test/java/org/apache/drill/TestExampleQueries.java > 225b21e > exec/java-exec/src/test/java/org/apache/drill/TestUnionAll.java 36b062b > exec/java-exec/src/test/resources/store/text/data/t.json PRE-CREATION > exec/java-exec/src/test/resources/testframework/TestUnionAll/q1.tsv > PRE-CREATION > exec/java-exec/src/test/resources/testframework/TestUnionAll/q10.tsv > PRE-CREATION > exec/java-exec/src/test/resources/testframework/TestUnionAll/q11.tsv > PRE-CREATION > exec/java-exec/src/test/resources/testframework/TestUnionAll/q12.tsv > PRE-CREATION > exec/java-exec/src/test/resources/testframework/TestUnionAll/q13.tsv > PRE-CREATION > exec/java-exec/src/test/resources/testframework/TestUnionAll/q14.tsv > PRE-CREATION > exec/java-exec/src/test/resources/testframework/TestUnionAll/q15.tsv > PRE-CREATION > exec/java-exec/src/test/resources/testframework/TestUnionAll/q2.tsv > PRE-CREATION > exec/java-exec/src/test/resources/testframework/TestUnionAll/q3.tsv > PRE-CREATION > exec/java-exec/src/test/resources/testframework/TestUnionAll/q4.tsv > PRE-CREATION > exec/java-exec/src/test/resources/testframework/TestUnionAll/q5.tsv > PRE-CREATION > exec/java-exec/src/test/resources/testframework/TestUnionAll/q6.tsv > PRE-CREATION > exec/java-exec/src/test/resources/testframework/TestUnionAll/q6_1.tsv > PRE-CREATION > exec/java-exec/src/test/resources/testframework/TestUnionAll/q7.tsv > PRE-CREATION > exec/java-exec/src/test/resources/testframework/TestUnionAll/q8.tsv > PRE-CREATION > exec/java-exec/src/test/resources/testframework/TestUnionAll/q9.tsv > PRE-CREATION > > exec/java-exec/src/test/resources/testframework/testExampleQueries/testAggregationOnUnionAllOperator/q1.tsv > PRE-CREATION > > exec/java-exec/src/test/resources/testframework/testExampleQueries/testAggregationOnUnionAllOperator/q2.tsv > PRE-CREATION > > Diff: https://reviews.apache.org/r/31707/diff/ > > > Testing > ------- > > Design Doc can be found from: > https://issues.apache.org/jira/browse/DRILL-2207 > > Unit, Customers, TPCH passed > waiting for Functional... > > > Thanks, > > Sean Hsuan-Yi Chu > >
