[ https://issues.apache.org/jira/browse/HIVE-4502?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13709352#comment-13709352 ]
Phabricator commented on HIVE-4502: ----------------------------------- ashutoshc has requested changes to the revision "HIVE-4502 [jira] NPE - subquery smb joins fails". I dont like the idea of tracking operators in the task thereby tasks getting involved in plan manipulation but in absence of alternatives, this may be the way to move forward for now. INLINE COMMENTS ql/src/java/org/apache/hadoop/hive/ql/QueryPlan.java:619 I guess this is unintentional. ql/src/java/org/apache/hadoop/hive/ql/exec/Task.java:72 I am little hesistant about task being aware of operators. Ideally if we have good demarkation of different layers, task (which is sort of a driver to execute a plan) need not be aware of operators contained in its plan. Planner shouldn't wait so late that it needs to track operators in task to generate plan correctly. I don't have a proposal on how to do this better though. At the very least, it will help to add comment explaining the purpose of this list. ql/src/java/org/apache/hadoop/hive/ql/optimizer/GenMRRedSink1.java:70 I think its better to do following: If (op.getNumChild() != 1){ throw new IllegalStateException("Expecting operator " + op + "to have one child. Found: " + op.getNumChild()); } ql/src/java/org/apache/hadoop/hive/ql/optimizer/GenMapRedUtils.java:261 It will be good to add comments on what this method intends to do. Also, I guess you can make this protected, since all the callers of this method are in same package. ql/src/java/org/apache/hadoop/hive/ql/optimizer/GenMapRedUtils.java:281 Does this method need to be public? ql/src/java/org/apache/hadoop/hive/ql/optimizer/GenMapRedUtils.java:310 I didnt get this part. Can you add comments why old and curr tasks need to be of type ExecDriver. What about MapredLocalWork? ql/src/java/org/apache/hadoop/hive/ql/optimizer/GenMapRedUtils.java:318 Does this method need to be public? Also, please add javadocs for the method. ql/src/java/org/apache/hadoop/hive/ql/optimizer/GenMapRedUtils.java:851 throw IllegalStateException in case # of parents != 1 ql/src/java/org/apache/hadoop/hive/ql/optimizer/GenMapRedUtils.java:949 I didnt get why did you remove this if check. We shall be needing tagging only for joins. Right? Removing this check implies we will tag always, is that needed? ql/src/java/org/apache/hadoop/hive/ql/optimizer/GenMapRedUtils.java:942 Better name: hasBranchFinished() ? ql/src/java/org/apache/hadoop/hive/ql/optimizer/MapJoinFactory.java:228 Why is this function call needed here? Can you please add comment for it? ql/src/test/results/clientpositive/auto_smb_mapjoin_14.q.out:165 I see that there is no Stage-2 at all. What caused this? Its little weird that we have stages 0,1 and 3 in plan and no stage-2 at all. ql/src/test/results/clientpositive/auto_smb_mapjoin_14.q.out:349 No stage-2 here either. Probably the same cause? REVISION DETAIL https://reviews.facebook.net/D10695 BRANCH HIVE-4502 ARCANIST PROJECT hive To: JIRA, ashutoshc, navis Cc: brock > NPE - subquery smb joins fails > ------------------------------ > > Key: HIVE-4502 > URL: https://issues.apache.org/jira/browse/HIVE-4502 > Project: Hive > Issue Type: Bug > Components: Query Processor > Affects Versions: 0.11.0 > Reporter: Vikram Dixit K > Assignee: Navis > Attachments: HIVE-4502.D10695.1.patch, HIVE-4502.D10695.2.patch, > HIVE-4502.D10695.3.patch, smb_mapjoin_25.q, smb_mapjoin_25.q > > > Found this issue while running some SMB joins. Attaching test case that > causes this error. -- 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