[GitHub] drill pull request #1059: DRILL-5851: Empty table during a join operation wi...

[paul-rogers]

Github user paul-rogers commented on a diff in the pull request:

    https://github.com/apache/drill/pull/1059#discussion_r155939344
  
    --- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/MergeJoinBatch.java
 ---
    @@ -152,7 +152,10 @@ public void buildSchema() {
           return;
         }
     
    -    if (leftOutcome == IterOutcome.NONE && rightOutcome == 
IterOutcome.NONE) {
    +    if (joinType == JoinRelType.INNER && (leftOutcome == IterOutcome.NONE 
|| rightOutcome == IterOutcome.NONE) ||
    +        joinType != JoinRelType.INNER && (leftOutcome == IterOutcome.NONE 
&& rightOutcome == IterOutcome.NONE)) {
    +      drainStream(leftOutcome, 0, left);
    +      drainStream(rightOutcome, 1, right);
    --- End diff --
    
    Does this do what it sounds like it does? Read values until there are no 
more to read? I wonder if this has been fully tested, or if it will end up 
running the subquery to completion unnecessarily?
    
    Also, here we are checking for NONE. Above we checked for error codes. 
Should we check for the error codes here?
    
    Or, better, when we receive an error code, should we simply throw an 
exception and end it all? (That is, the code does not currently do anything 
useful with STOP, OUT_OF_MEMORY or NOTYET.)


---