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