C++: Writing sliced record batch to IPC writes the entire array

[asfimport]

The bug can be triggered through python:

import pyarrow.parquet
array = pyarrow.array.from_pylist([1] * 1000000)

rb = pyarrow.RecordBatch.from_arrays([array], ['a'])
rb2 = rb.slice(0,2)

with open('/tmp/t.arrow', 'wb') as f:
  w = pyarrow.ipc.FileWriter(f, rb.schema)
  w.write_batch(rb2)
  w.close()

which will result in a big file:

$ ll /tmp/t.arrow 
-rw-rw-r-- 1 itai itai 800618 Apr 12 13:22 /tmp/t.arrow

Reporter: Itai Incze / @itaiin
Assignee: Wes McKinney / @wesm

Related issues:

• Arrow 0.3 release (blocks)

_{Note: This issue was originally created as ARROW-809. Please see the migration documentation for further details.}

[asfimport]

Itai Incze / @itaiin:
The reason for the bug seems to be that sliced arrays are tested with the condition (array.offset() != 0) throughout arrow/ipc/writer.cc, which doesn't account for a \[0:x\] slice.

To test for 0-based slices there's a need to know the number of elements in the original array, which is easily computed in cases like fixed-width array types but could be harder in others.

Another approach could be adding a class member to mark the slices. This could be either by a simple boolean, the original dimensions or a reference to the original array.

[asfimport]

Wes McKinney / @wesm:
Marked for 0.3. I don't think this should be hard to fix

[asfimport]

Wes McKinney / @wesm:
I'm going to truncate the data buffers to a 64-byte padding offset, patch coming tomorrow probably

[asfimport]

Itai Incze / @itaiin:
I've fiddled with it a bit - without altering the array class, I found there's a problem finding the exact number of items with a boolean array - where it doesnt matter, and in union array. There may be other instances as well that i'm not aware of.

Seems to me that adding a private boolean IsSliced to the array is the cleanest way.

[asfimport]

Wes McKinney / @wesm:
There is some buffer slicing happening on the IPC write path already: https://github.com/apache/arrow/blob/master/cpp/src/arrow/ipc/writer.cc#L207. It needs to be made consistent (+ well tested), though (and work for sliced arrays / offset 0)

[asfimport]

Itai Incze / @itaiin:
I wrote the comment before seeing yours latest one, so not in order to doubt the solution.
I've seen that code... though I'm certain you're much better acquainted with it than I am :)

[asfimport]

Wes McKinney / @wesm:
PR (WIP): #555

[asfimport]

Wes McKinney / @wesm:
Issue resolved by pull request 555
#555