[GitHub] [drill] oleg-zinovev commented on pull request #2183: DRILL-6547: IllegalStateException: Tried to remove unmanaged buffer in concat function

[GitBox <gi...@apache.org>]

oleg-zinovev commented on pull request #2183:
URL: https://github.com/apache/drill/pull/2183#issuecomment-791903981


   @paul-rogers 
   Thank you so much for parsing the fix.
   
   1. Here are the code generated to perform the projection:
   ```java
   public class ProjectorGen0
       extends ProjectorTemplate
   {
   
       DrillBuf work0;
       NullableVarCharHolder constant2;
       NullableVarCharVector vv3;
       VarCharVector vv8;
   
       public ProjectorGen0() {
           try {
               __DRILL_INIT__();
           } catch (SchemaChangeException e) {
               throw new UnsupportedOperationException(e);
           }
       }
   
       public void doEval(int inIndex, int outIndex)
           throws SchemaChangeException
       {
           {
               NullableVarCharHolder out6 = new NullableVarCharHolder();
               {
                   out6 .isSet = vv3 .getAccessor().isSet((inIndex));
                   if (out6 .isSet == 1) {
                       out6 .buffer = vv3 .getBuffer();
                       long startEnd = vv3 .getAccessor().getStartEnd((inIndex));
                       out6 .start = ((int) startEnd);
                       out6 .end = ((int)(startEnd >> 32));
                   }
               }
               //---- start of eval portion of concat function. ----//
               VarCharHolder out7 = new VarCharHolder();
               {
                   final VarCharHolder out = new VarCharHolder();
                   NullableVarCharHolder left = constant2;
                   NullableVarCharHolder right = out6;
                   DrillBuf buffer = work0;
                   StringFunctions$ConcatBothNullInput_eval:
   {
       out.buffer = buffer = buffer.reallocIfNeeded((left.end - left.start) + (right.end - right.start));
       out.start = out.end = 0;
   
       int id = 0;
   
       if (left.isSet == 1) {
           for (id = left.start; id < left.end; id++) {
               out.buffer.setByte(out.end++, left.buffer.getByte(id));
           }
       }
       if (right.isSet == 1) {
           for (id = right.start; id < right.end; id++) {
               out.buffer.setByte(out.end++, right.buffer.getByte(id));
           }
       }
   }
                   work0 = buffer;
                   out7 .start = out.start;
                   out7 .end = out.end;
                   out7 .buffer = out.buffer;
               }
               //---- end of eval portion of concat function. ----//
               vv8 .getMutator().setSafe((outIndex), out7 .start, out7 .end, out7 .buffer);
           }
       }
   
       public void doSetup(FragmentContext context, RecordBatch incoming, RecordBatch outgoing)
           throws SchemaChangeException
       {
           {
               work0 = (incoming).getContext().getManagedBuffer();
               NullableVarCharHolder out1 = new NullableVarCharHolder();
               constant2 = new NullableVarCharHolder();
               constant2 .isSet = out1 .isSet;
               constant2 .start = out1 .start;
               constant2 .end = out1 .end;
               constant2 .buffer = out1 .buffer;
               int[] fieldIds4 = new int[ 1 ] ;
               fieldIds4 [ 0 ] = 0;
               Object tmp5 = (incoming).getValueAccessorById(NullableVarCharVector.class, fieldIds4).getValueVector();
               if (tmp5 == null) {
                   throw new SchemaChangeException("Failure while loading vector vv3 with id: TypedFieldId [fieldIds=[0], remainder=null].");
               }
               vv3 = ((NullableVarCharVector) tmp5);
               /** start SETUP for function concat **/ 
               {
                   NullableVarCharHolder left = constant2;
                   DrillBuf buffer = work0;
                   StringFunctions$ConcatBothNullInput_setup:
   {}
                   work0 = buffer;
               }
               /** end SETUP for function concat **/ 
               int[] fieldIds9 = new int[ 1 ] ;
               fieldIds9 [ 0 ] = 0;
               Object tmp10 = (outgoing).getValueAccessorById(VarCharVector.class, fieldIds9).getValueVector();
               if (tmp10 == null) {
                   throw new SchemaChangeException("Failure while loading vector vv8 with id: TypedFieldId [fieldIds=[0], remainder=null].");
               }
               vv8 = ((VarCharVector) tmp10);
           }
       }
   
       public void __DRILL_INIT__()
           throws SchemaChangeException
       {
       }
   
   }
   ```
   
   As you can see, the buffer is requested from the fragment context in the doSetup method (which in turn is called within setup). This happens once for each new schema in **ProjectRecordBatch#setupNewSchemaFromInput**
   
   When calculating the function body, the buffer usage pattern is as follows:
   1) Extract the current buffer from the class field to a local variable
   2) Execute the function body
   3) Save the value of the local buffer variable in the class field
   
   In the current implementation, managed buffer is "reused" for each subsequent row. Therefore, the result of reallocIfNeeded must be stored in the buffer field declared within the function.
   
   2. You are not quite right when you say that the result of left. end - left. start will always be 0. For example, the same implementation of the function will be used in the case when the first argument is the nullable field of the parquet file, in which not all values will be null.
   P.S. Perhaps I did not understand you correctly
   
   3. Unfortunately, I can't use the sample request given in the bug, as it uses a file that is missing from the drill project. In addition, this file is uploaded to MapR jira, where I do not have access. If you can attach a file to the specified Drill bug, I will happily replace my request with the request given in the bug.
   
   P.S. I'm sorry for my English 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org