[ https://issues.apache.org/jira/browse/HIVE-3562?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13749290#comment-13749290 ]
Phabricator commented on HIVE-3562: ----------------------------------- navis has commented on the revision "HIVE-3562 [jira] Some limit can be pushed down to map stage". Thanks for a review. INLINE COMMENTS common/src/java/org/apache/hadoop/hive/conf/HiveConf.java:528 I thought it's a convention of hive to make configurations for enable/disable and threshold each, which was requested several times till now for me for other issues. I'll address that. ql/src/java/org/apache/hadoop/hive/ql/exec/GroupByOperator.java:1187 Legend : A(a) --> key A, value a, row A(a) If each mapper takes rows lite this MAP1(RS) : 40(a)-10(b)-30(c)-10(d)-70(e)-80(f) MAP2(RS) : 90(g)-80(h)-60(i)-40(j)-30(k)-20(l) MAP3(RS) : 40(m)-50(n)-30(o)-30(p)-60(q)-70(r) REDUCER : 10(b,d)-20(l)-30(c,k,o)-40(a,j,m)-50(n)-60(i,q)-70(e,r)-80(f,h)-90(g) REDUCER(LIMIT 3) : 10(b,d)-20(l)-30(c,k,o) with this patch MAP1 : 40(a)-10(b)-30(c)-10(d) MAP2 : 40(j)-30(k)-20(l) MAP3 : 40(m)-50(n)-30(o)-30(p) REDUCER : 10(b,d)-20(l)-30(c,k,o)-40(a,j,m)-50(n) REDUCER(LIMIT 3) : 10(b,d)-20(l)-30(c,k,o) ql/src/java/org/apache/hadoop/hive/ql/exec/ReduceSinkOperator.java:154 I thought limit 0 is not realistic and can be used for some kind of testing. I'll address that case but making Hash seemed a little much. ql/src/java/org/apache/hadoop/hive/ql/exec/ReduceSinkOperator.java:157 ReduceSinkHash makes arrays for key/value/hashes. It's for compensating those (not exact but I like the number). ql/src/java/org/apache/hadoop/hive/ql/exec/ReduceSinkOperator.java:325 I think there was a test for RS operator which has no collector. I'll check that. ql/src/java/org/apache/hadoop/hive/ql/exec/ReduceSinkOperator.java:340 Right. I've missed that. ql/src/java/org/apache/hadoop/hive/ql/exec/ReduceSinkOperator.java:343 It's possible to add key/value to hash which is flushed right before. But I just forwarded it. ql/src/java/org/apache/hadoop/hive/ql/exec/ReduceSinkOperator.java:346 Right. ql/src/java/org/apache/hadoop/hive/ql/exec/ReduceSinkOperator.java:434-436 It's not serializable class. But I'll add transient mod. ql/src/java/org/apache/hadoop/hive/ql/exec/ReduceSinkOperator.java:471 I'll leave log message in the processOp() method. ql/src/java/org/apache/hadoop/hive/ql/exec/ReduceSinkOperator.java:477 Return value can be false only if it's for GBY. False means the key exists already in RS hash. For case A(a)->A(b)->B(c) with limit 5, the first A(a) will be stored into hash. For the second A(b), value b should be stored, which means values should be byte[][][] using additional array for each index consuming more memory. But I suspected that case could be a common case with map aggregation. So I just decided to forward A(b) to output collector. ql/src/java/org/apache/hadoop/hive/ql/exec/ReduceSinkOperator.java:495 It does not remove anything. This is called after index is removed from RS hash. ql/src/java/org/apache/hadoop/hive/ql/exec/ReduceSinkOperator.java:498 Good. ql/src/java/org/apache/hadoop/hive/ql/exec/ReduceSinkOperator.java:535 Main difference of HashForOrder from HashForGroupBy is that it should store duplicated keys. As described in comments, A->B->B->C limit 3 should store A,B,B not A,B,C. I believe this behavior can be possibly implemented with simple TreeSet but thought this is way simpler. ql/src/java/org/apache/hadoop/hive/ql/exec/ReduceSinkOperator.java:566 I prefer Map rather than Set but I'll change that. ql/src/test/queries/clientpositive/limit_pushdown.q:14 ok ql/src/test/queries/clientpositive/limit_pushdown.q:43 ok. ql/src/java/org/apache/hadoop/hive/ql/optimizer/LimitPushdownOptimizer.java:46 This is not exact description. RS for limit (not last RS). I'll change this, too. ql/src/java/org/apache/hadoop/hive/ql/exec/ReduceSinkOperator.java:253-264 Looks promising. Need more investigation on implementation of distinct. REVISION DETAIL https://reviews.facebook.net/D5967 BRANCH HIVE-3562 ARCANIST PROJECT hive To: JIRA, tarball, ashutoshc, navis Cc: njain > Some limit can be pushed down to map stage > ------------------------------------------ > > Key: HIVE-3562 > URL: https://issues.apache.org/jira/browse/HIVE-3562 > Project: Hive > Issue Type: Bug > Reporter: Navis > Assignee: Navis > Priority: Trivial > Attachments: HIVE-3562.D5967.1.patch, HIVE-3562.D5967.2.patch, > HIVE-3562.D5967.3.patch, HIVE-3562.D5967.4.patch, HIVE-3562.D5967.5.patch, > HIVE-3562.D5967.6.patch > > > Queries with limit clause (with reasonable number), for example > {noformat} > select * from src order by key limit 10; > {noformat} > makes operator tree, > TS-SEL-RS-EXT-LIMIT-FS > But LIMIT can be partially calculated in RS, reducing size of shuffling. > TS-SEL-RS(TOP-N)-EXT-LIMIT-FS -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira