impala-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Tim Armstrong (Code Review)" <ger...@cloudera.org>
Subject [Impala-CR](cdh5-trunk) IMPALA-2502: don't redundantly repartition grouping aggregations
Date Mon, 07 Mar 2016 22:18:25 GMT
Tim Armstrong has posted comments on this change.

Change subject: IMPALA-2502: don't redundantly repartition grouping aggregations
......................................................................


Patch Set 4:

(8 comments)

http://gerrit.cloudera.org:8080/#/c/2414/4/fe/src/main/java/com/cloudera/impala/planner/DataPartition.java
File fe/src/main/java/com/cloudera/impala/planner/DataPartition.java:

Line 47:   public DataPartition(TPartitionType type, List<Expr> exprs) {
> let's make this private now
Done


http://gerrit.cloudera.org:8080/#/c/2414/4/fe/src/main/java/com/cloudera/impala/planner/DistributedPlanner.java
File fe/src/main/java/com/cloudera/impala/planner/DistributedPlanner.java:

Line 743:       throws ImpalaException, InternalException {
> ImpalaException should be sufficient
Done


Line 756:       boolean childPartitionEquivalent = ctx_.getRootAnalyzer().equivSets(partitionExprs,
> Would be nice to keep the var naming consistent within this file. For the j
Done


Line 760:         // the aggregation in the child fragment.
> .. in the child fragment without an extra merge step.
Done


Line 809:       throws ImpalaException, InternalException {
> ImpalaException should be sufficient
Done


http://gerrit.cloudera.org:8080/#/c/2414/4/testdata/workloads/functional-planner/queries/PlannerTest/aggregation.test
File testdata/workloads/functional-planner/queries/PlannerTest/aggregation.test:

Line 920: from tpch_parquet.customer inner join [shuffle] tpch_parquet.orders on c_custkey
= o_custkey
> add a having clause and a limit to make sure those work correctly
Done


Line 947: select straight_join count(distinct c_custkey)
> Add or modify an existing test where there are interesting expressions on t
I'm not sure that I understand which slots and what would qualify as interesting expressions


Line 979: select straight_join c_custkey, count(distinct c_custkey)
> Add a new more complicated test that combined two joins and an aggregation 
Done


-- 
To view, visit http://gerrit.cloudera.org:8080/2414
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iffdcfd3629b8a69bd23915e1adba3b8323cbbaef
Gerrit-PatchSet: 4
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-Reviewer: Alex Behm <alex.behm@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <marcel@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message