You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Henry Robinson (Code Review)" <ge...@cloudera.org> on 2016/10/21 20:57:29 UTC
[Impala-ASF-CR] IMPALA-4212: Add sink output expressions to explain output
Henry Robinson has uploaded a new change for review.
http://gerrit.cloudera.org:8080/4783
Change subject: IMPALA-4212: Add sink output expressions to explain output
......................................................................
IMPALA-4212: Add sink output expressions to explain output
* Move output exprs from plan fragment to sink
* Add OUTPUT-EXPRS=(...) to relevant sinks
Test results attached. Some spurious changes not yet removed, will tidy
up once format is agreed.
The explain plan can have very long lines. IMPALA-4337 is the (open)
issue to address this.
Change-Id: I27c3a47d7ee28a9efb1405dd1a6ef9b7a83931f6
---
M be/src/benchmarks/expr-benchmark.cc
M be/src/exec/data-sink.cc
M be/src/exec/data-sink.h
M be/src/exec/hbase-table-sink.cc
M be/src/exec/hbase-table-sink.h
M be/src/exec/hdfs-table-sink.cc
M be/src/exec/hdfs-table-sink.h
M be/src/exec/kudu-table-sink.cc
M be/src/exec/kudu-table-sink.h
M be/src/exec/plan-root-sink.cc
M be/src/exec/plan-root-sink.h
M be/src/runtime/plan-fragment-executor.cc
M common/thrift/DataSinks.thrift
M common/thrift/Planner.thrift
M fe/src/main/java/org/apache/impala/planner/DataSink.java
M fe/src/main/java/org/apache/impala/planner/DataStreamSink.java
M fe/src/main/java/org/apache/impala/planner/HBaseTableSink.java
M fe/src/main/java/org/apache/impala/planner/HdfsTableSink.java
M fe/src/main/java/org/apache/impala/planner/JoinBuildSink.java
M fe/src/main/java/org/apache/impala/planner/KuduTableSink.java
M fe/src/main/java/org/apache/impala/planner/PlanFragment.java
M fe/src/main/java/org/apache/impala/planner/PlanRootSink.java
M fe/src/main/java/org/apache/impala/planner/Planner.java
M testdata/workloads/functional-planner/queries/PlannerTest/aggregation.test
M testdata/workloads/functional-planner/queries/PlannerTest/analytic-fns.test
M testdata/workloads/functional-planner/queries/PlannerTest/complex-types-file-formats.test
M testdata/workloads/functional-planner/queries/PlannerTest/conjunct-ordering.test
M testdata/workloads/functional-planner/queries/PlannerTest/constant.test
M testdata/workloads/functional-planner/queries/PlannerTest/data-source-tables.test
M testdata/workloads/functional-planner/queries/PlannerTest/ddl.test
M testdata/workloads/functional-planner/queries/PlannerTest/disable-preaggregations.test
M testdata/workloads/functional-planner/queries/PlannerTest/distinct-estimate.test
M testdata/workloads/functional-planner/queries/PlannerTest/distinct.test
M testdata/workloads/functional-planner/queries/PlannerTest/empty.test
M testdata/workloads/functional-planner/queries/PlannerTest/hbase.test
M testdata/workloads/functional-planner/queries/PlannerTest/hdfs.test
M testdata/workloads/functional-planner/queries/PlannerTest/implicit-joins.test
M testdata/workloads/functional-planner/queries/PlannerTest/inline-view-limit.test
M testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test
M testdata/workloads/functional-planner/queries/PlannerTest/insert.test
M testdata/workloads/functional-planner/queries/PlannerTest/join-order.test
M testdata/workloads/functional-planner/queries/PlannerTest/joins.test
M testdata/workloads/functional-planner/queries/PlannerTest/kudu-delete.test
M testdata/workloads/functional-planner/queries/PlannerTest/kudu-selectivity.test
M testdata/workloads/functional-planner/queries/PlannerTest/kudu-update.test
M testdata/workloads/functional-planner/queries/PlannerTest/lineage.test
M testdata/workloads/functional-planner/queries/PlannerTest/mem-limit-broadcast-join.test
M testdata/workloads/functional-planner/queries/PlannerTest/mt-dop-validation.test
M testdata/workloads/functional-planner/queries/PlannerTest/nested-collections.test
M testdata/workloads/functional-planner/queries/PlannerTest/nested-loop-join.test
M testdata/workloads/functional-planner/queries/PlannerTest/order.test
M testdata/workloads/functional-planner/queries/PlannerTest/outer-joins.test
M testdata/workloads/functional-planner/queries/PlannerTest/partition-key-scans.test
M testdata/workloads/functional-planner/queries/PlannerTest/predicate-propagation.test
M testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-propagation.test
M testdata/workloads/functional-planner/queries/PlannerTest/small-query-opt.test
M testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test
M testdata/workloads/functional-planner/queries/PlannerTest/topn.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpcds-all.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpch-all.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpch-nested.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpch-views.test
M testdata/workloads/functional-planner/queries/PlannerTest/union.test
M testdata/workloads/functional-planner/queries/PlannerTest/values.test
M testdata/workloads/functional-planner/queries/PlannerTest/views.test
M testdata/workloads/functional-planner/queries/PlannerTest/with-clause.test
66 files changed, 1,779 insertions(+), 1,762 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/83/4783/1
--
To view, visit http://gerrit.cloudera.org:8080/4783
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I27c3a47d7ee28a9efb1405dd1a6ef9b7a83931f6
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
[Impala-ASF-CR] IMPALA-4212: Add sink output expressions to explain output
Posted by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org>.
Thomas Tauber-Marshall has posted comments on this change.
Change subject: IMPALA-4212: Add sink output expressions to explain output
......................................................................
Patch Set 1:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/4783/1/fe/src/main/java/org/apache/impala/planner/DataSink.java
File fe/src/main/java/org/apache/impala/planner/DataSink.java:
Line 34: import java.util.ArrayList;
Looks like you don't actually use several of these - ArrayList, Splitter, WordUtils.
Line 77: outputExprs_ = outputExprs;
single line
http://gerrit.cloudera.org:8080/#/c/4783/1/fe/src/main/java/org/apache/impala/planner/PlanRootSink.java
File fe/src/main/java/org/apache/impala/planner/PlanRootSink.java:
Line 34: return String.format("PLAN-ROOT SINK [%s]\n", getOutputExprsString());
I guess you're ignoring prefix because its always going to be an empty string here? Maybe note that.
Line 38: return initThriftSink(TDataSinkType.PLAN_ROOT_SINK);
single line
--
To view, visit http://gerrit.cloudera.org:8080/4783
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I27c3a47d7ee28a9efb1405dd1a6ef9b7a83931f6
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <ma...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4212: Add sink output expressions to explain output
Posted by "Marcel Kornacker (Code Review)" <ge...@cloudera.org>.
Marcel Kornacker has posted comments on this change.
Change subject: IMPALA-4212: Add sink output expressions to explain output
......................................................................
Patch Set 1:
(5 comments)
looks good, but let's only print the output exprs in explain for explain level 'verbose'.
http://gerrit.cloudera.org:8080/#/c/4783/1/be/src/exec/hdfs-table-sink.cc
File be/src/exec/hdfs-table-sink.cc:
Line 65: tsink.table_sink.hdfs_table_sink.__isset.skip_header_line_count ?
if you really want to break this up, then please as
...\n
? ...\n
: 0
http://gerrit.cloudera.org:8080/#/c/4783/1/common/thrift/DataSinks.thrift
File common/thrift/DataSinks.thrift:
Line 104: // Exprs that produce values for slots of output tuple (one expr per slot).
move this right below type, we typically have the common fields precede the 'union' fields
http://gerrit.cloudera.org:8080/#/c/4783/1/fe/src/main/java/org/apache/impala/planner/DataSink.java
File fe/src/main/java/org/apache/impala/planner/DataSink.java:
Line 91: protected String getOutputExprsString() {
getOutputExprsExplainString()?
Line 95: for (Expr e : outputExprs_) {
single line
http://gerrit.cloudera.org:8080/#/c/4783/1/fe/src/main/java/org/apache/impala/planner/HdfsTableSink.java
File fe/src/main/java/org/apache/impala/planner/HdfsTableSink.java:
Line 117: targetTable_.getFullName(), overwriteStr, partitionKeyStr, getOutputExprsString()));
long line
--
To view, visit http://gerrit.cloudera.org:8080/4783
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I27c3a47d7ee28a9efb1405dd1a6ef9b7a83931f6
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <ma...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4212: Add sink output expressions to explain output
Posted by "Henry Robinson (Code Review)" <ge...@cloudera.org>.
Henry Robinson has abandoned this change.
Change subject: IMPALA-4212: Add sink output expressions to explain output
......................................................................
Abandoned
--
To view, visit http://gerrit.cloudera.org:8080/4783
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: abandon
Gerrit-Change-Id: I27c3a47d7ee28a9efb1405dd1a6ef9b7a83931f6
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <ma...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>