[
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