[jira] [Commented] (HIVE-3980) Cleanup after HIVE-3403

["Ashutosh Chauhan (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/HIVE-3980?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13591601#comment-13591601 ] 

Ashutosh Chauhan commented on HIVE-3980:
----------------------------------------

>From the list you posted on this jira, I don't see any code changes/code comments/explanation for following changes:

114 ↗
(On Diff #26877)	
I think it makes more sense to do this check before looping over and comparing orders, since this is a stricter check, in a sense that if all join keys are not bucketed columns there is no point in checking order.

Also can you refactor this to return List<Path> instead, we had troubles because of String Vs Path interchangeable usage in Hive code base in past.

Actually, it seems that you can always derive this info from the next field, it is just the size of the innermost list in next map. If so, please consider getting rid of this map and just use next field wherever this one is required. 
Having too many datastructures makes code harder to follow as well as results in tighter function coupling.

200–203 ↗
(On Diff #26877)	
Similar comment as the one above applies here as well.

(On Diff #26877)	
Should this be topOpEntry.getValue().equals(tso) instead ?

On Diff #26877)	
If table is not sorted, we cannot do sortBMJ anyways. Secondly we cannot do sortBMJ for outer joins either. Shouldn't this check be made much earlier?

Early parts of this method performs some of the same checks of checkConvertBucketMapJoin() of AbstractBucketJoinProc.java. Perhaps, we can refactor to avoid code duplication.
                
> Cleanup after HIVE-3403
> -----------------------
>
>                 Key: HIVE-3980
>                 URL: https://issues.apache.org/jira/browse/HIVE-3980
>             Project: Hive
>          Issue Type: Bug
>          Components: Query Processor
>            Reporter: Namit Jain
>            Assignee: Namit Jain
>         Attachments: hive.3980.1.patch, hive.3980.2.patch, hive.3980.3.patch
>
>
> There have been a lot of comments on HIVE-3403, which involve changing 
> variable names/function names/adding more comments/general cleanup etc.
> Since HIVE-3403 involves a lot of refactoring, it was fairly difficult to
> address the comments there, since refreshing becomes impossible. This jira
> is to track those cleanups.

--
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