drill-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jinfengni <...@git.apache.org>
Subject [GitHub] drill pull request #584: DRILL-4884: Fix bug that drill sometimes produced I...
Date Mon, 24 Oct 2016 23:02:17 GMT
Github user jinfengni commented on a diff in the pull request:

    https://github.com/apache/drill/pull/584#discussion_r84801937
  
    --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/validate/IteratorValidatorBatchIterator.java
---
    @@ -301,7 +301,7 @@ public IterOutcome next() {
                       "Incoming batch [#%d, %s] has an empty schema. This is not allowed.",
                       instNum, batchTypeName));
             }
    -        if (incoming.getRecordCount() > MAX_BATCH_SIZE) {
    +        if (incoming.getRecordCount() >= MAX_BATCH_SIZE) {
    --- End diff --
    
    I'm not sure if this is the right fix for this IOB problem. 
    
    1. IteratorValidator is only inserted when assertion is enabled [1].  Fixing only in IteratorValidatorBatchIterator
will mean that the issue will be still there if assertion is disabled.
    2. Even we do turn on assertion,  will the query hit IllegalStateException, in stead of
IOB?
    
    Can you try run the query when assertion is off / on, and see if the query is successful
in both cases?
    
    [1] https://github.com/apache/drill/blob/master/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ImplCreator.java#L72-L74
     


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

Mime
View raw message