[GitHub] [arrow] eerhardt commented on pull request #10562: ARROW-13129: [C#] Fix TableFromRecordBatches

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

eerhardt commented on pull request #10562:
URL: https://github.com/apache/arrow/pull/10562#issuecomment-875069925


   Alternatively, we could change `TableFromRecordBatches` to be:
   
   ```C#
            public static Table TableFromRecordBatches(Schema schema, IList<RecordBatch> recordBatches)
           {
               int nBatches = recordBatches.Count;
               int nColumns = schema.Fields.Count;
   
               List<Column> columns = new List<Column>(nColumns);
               for (int icol = 0; icol < nColumns; icol++)
               {
                   List<Array> columnArrays = new List<Array>(nBatches);
                   for (int jj = 0; jj < nBatches; jj++)
                   {
                       columnArrays.Add(recordBatches[jj].Column(icol) as Array);
                   }
                   columns.Add(new Arrow.Column(schema.GetFieldByIndex(icol), columnArrays));
               }
   ```
   
   And undo the `ToList()` changes here.
   
   The reason I'd prefer that approach is because we have a bunch of other APIs that assume they take ownership of the array/list that is passed to them. The reason for that is to limit the amount of allocations. If someone builds up a single list, passes it into the `Table` constructor, and then moves on, it is a waste for the Table constructor to make a copy of the list as well. It is confusing for some APIs to make a defensive copy, but others not to.
   
   Thoughts?


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

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

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