liuyao has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/17821 )

Change subject: IMPALA-2581: LIMIT can be propagated down into some aggregations
......................................................................


Patch Set 13:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/17821/12//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/17821/12//COMMIT_MSG@10
PS12, Line 10:
> nit. exceed the max line threshold.
Done


http://gerrit.cloudera.org:8080/#/c/17821/12/be/src/exec/aggregation-node.cc
File be/src/exec/aggregation-node.cc:

http://gerrit.cloudera.org:8080/#/c/17821/12/be/src/exec/aggregation-node.cc@76
PS12, Line 76:  VLOG_QUERY << Substitute("the number of rows ($0) returned from 
the aggregation"
             :           " node has exceeded the limit of $1", 
aggs_[0]->GetNumKeys(), limit());
> nit. May use Substitute() which is faster.
Done


http://gerrit.cloudera.org:8080/#/c/17821/12/common/thrift/PlanNodes.thrift
File common/thrift/PlanNodes.thrift:

http://gerrit.cloudera.org:8080/#/c/17821/12/common/thrift/PlanNodes.thrift@479
PS12, Line 479: compl
> nit. complete
Done


http://gerrit.cloudera.org:8080/#/c/17821/12/fe/src/main/java/org/apache/impala/planner/AggregationNode.java
File fe/src/main/java/org/apache/impala/planner/AggregationNode.java:

http://gerrit.cloudera.org:8080/#/c/17821/12/fe/src/main/java/org/apache/impala/planner/AggregationNode.java@646
PS12, Line 646: When both conditions below are true, aggregati
> nit. I think we should mention the two conditions in the commit message her
Done


http://gerrit.cloudera.org:8080/#/c/17821/12/fe/src/main/java/org/apache/impala/planner/AggregationNode.java@648
PS12, Line 648: on n
> nit. May use Complete, as Halt implies stop for some reason.
Done


http://gerrit.cloudera.org:8080/#/c/17821/11/testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test
File 
testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test:

http://gerrit.cloudera.org:8080/#/c/17821/11/testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test@2934
PS11, Line 2934: limit: 2
> Since we do not push down id from the outer side to the inner, I would thin
where id = subquery,If this subQuery returns 2 rows, we can sure that it is not 
meet the semantic requirement, we should report a semantic error.We don't need 
to get all the results.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I930a6cb203615acfc03f23118d1bc1f0ea360995
Gerrit-Change-Number: 17821
Gerrit-PatchSet: 13
Gerrit-Owner: liuyao <liu...@sensorsdata.cn>
Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
Gerrit-Reviewer: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com>
Gerrit-Reviewer: liuyao <liu...@sensorsdata.cn>
Gerrit-Comment-Date: Mon, 13 Sep 2021 06:40:40 +0000
Gerrit-HasComments: Yes

Reply via email to