drill-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Paul Rogers (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (DRILL-5489) Unprotected array access in RepeatedVarCharOutput ctor
Date Tue, 09 May 2017 01:38:04 GMT

    [ https://issues.apache.org/jira/browse/DRILL-5489?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16001904#comment-16001904

Paul Rogers commented on DRILL-5489:

While we are at it, we might as well fix another bit of bogus code:

        for(Integer i : columnIds){
          maxField = 0;
          maxField = Math.max(maxField, i);

The code to compute maxField simply uses the last value, not the maximum value. This will
be thrown off by a query of the form:

SELECT columns[20], columns[1] FROM ...

This will muck up the text reader. The text reader "shortcuts" the rest of the line if it
need not read any more fields. In {{TextReader.parseRecord()}}:

        earlyTerm = ! parseField();
        if (earlyTerm) {
          if (ch != newLine) {


  private final boolean parseField() throws IOException {
      return output.endField();

And, in {{RepeatedVarCharOutput.endField()}}:

  public boolean endField() {
    return fieldIndex < maxField;

The only reason that this does not cause data loss is the insertion of an unnecessary sort
earlier in the constructor:

        for (Integer i : columnIds){

We can remove the sort and compute the maxField correct and get the same result with simpler

> Unprotected array access in RepeatedVarCharOutput ctor
> ------------------------------------------------------
>                 Key: DRILL-5489
>                 URL: https://issues.apache.org/jira/browse/DRILL-5489
>             Project: Apache Drill
>          Issue Type: Bug
>    Affects Versions: 1.10.0
>            Reporter: Paul Rogers
>            Priority: Minor
> Suppose a user runs a query of form:
> {code}
> SELECT columns[70000] FROM `dfs`.`mycsv.csv`
> {code}
> Internally, this will create a {{PathSegment}} to represent the selected column. This
is passed into the {{RepeatedVarCharOutput}} constructor where it is used to set a flag in
an array of 64K booleans. But, while the code is very diligent of making sure that the column
name is "columns" and that the path segment is an array, it does not check the array value.
> {code}
>         for(Integer i : columnIds){
>           ...
>           fields[i] = true;
>         }
> {code}
> We need to add a bounds check to reject array indexes that are not valid: negative or
above 64K. It may be that the code further up the hierarchy does the checks. But, if so, it
should do the other checks as well. Leaving the checks incomplete is confusing.
> The result:
> {code}
> Exception (no rows returned): org.apache.drill.common.exceptions.UserRemoteException:

> SYSTEM ERROR: ArrayIndexOutOfBoundsException: 70000
> {code}

This message was sent by Atlassian JIRA

View raw message