Jim Apple has posted comments on this change. Change subject: IMPALA-2581: LIMIT can be propagated down into some aggregations ......................................................................
Patch Set 4: (7 comments) http://gerrit.cloudera.org:8080/#/c/3822/3/be/src/exec/partitioned-aggregation-node.h File be/src/exec/partitioned-aggregation-node.h: PS3, Line 294: // > /// Done PS3, Line 298: max_groups_ is checked by comparing it to group_count_, th > Maybe a little misleading. I think the important check is in the -ir.cc fil Done Line 299: /// partitions. If the partitions hold more than max_groups_ groups, no new groups need > could be a little clearer: something like "limiting the number of groups in Done PS3, Line 671: > /// Done Line 675: /// that this function can return true when the rows have not been returned from this > I think the name is misleading because it's not really enforcing the same t Done http://gerrit.cloudera.org:8080/#/c/3822/3/fe/src/main/java/com/cloudera/impala/planner/DistributedPlanner.java File fe/src/main/java/com/cloudera/impala/planner/DistributedPlanner.java: PS3, Line 910: isPreaggLim > isPreaggLimitable()? Done http://gerrit.cloudera.org:8080/#/c/3822/3/testdata/workloads/functional-query/queries/QueryTest/spilling.test File testdata/workloads/functional-query/queries/QueryTest/spilling.test: Line 632: # Test spilling an agg with a LIMIT; see IMPALA-2581 > Were you able to reproduce the bug? I'd feel a lot more comfortable if we h Yes - with PS1 and PS2 do not work with this query -- To view, visit http://gerrit.cloudera.org:8080/3822 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I59c5b7af7a73ccdbc5496b28eacb9b6859d202bc Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Jim Apple <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Jim Apple <[email protected]> Gerrit-Reviewer: Matthew Jacobs <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-HasComments: Yes
