Binary statistics is not updated correctly if an underlying Binary array is modified in place

[asfimport]

The following test case shows the problem:

    byte[] bytes = new byte[] { 49 };
    BinaryStatistics reusableStats =  new BinaryStatistics();
    reusableStats.updateStats(Binary.fromByteArray(bytes));
    bytes[0] = 50;
    reusableStats.updateStats(Binary.fromByteArray(bytes, 0, 1));
 
    assertArrayEquals(new byte[] { 49 }, reusableStats.getMinBytes());
    assertArrayEquals(new byte[] { 50 }, reusableStats.getMaxBytes());

I discovered the bug when converting an AVRO file to a Parquet file by reading GenericRecords from a file using DataFileStream.next(D reuse) method. The problem is that underlying byte array of avro Utf8 object is passed to parquet that saves it as part of BinaryStatistics and then the same array is modified in place on the next read.

I am not sure what is the right way to fix the problem (in BinaryStatistics or AvroWriteSupport).

If BinaryStatistics implementation is correct (for performance reasons) then this behavior should be documented and AvroWriteSupport.fromAvroString should be fixed to duplicate underlying Utf8 array.

I am happy to create a pull request once the desired way to fix the issue is discussed.

Reporter: Konstantin Shaposhnikov / @kostya-sh

Related issues:

• Binary column statistics error when reuse byte[] among rows (duplicates)

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

[asfimport]

Ryan Blue / @rdblue:
I think this duplicates PARQUET-251. [~k.shaposhnikov@gmail.com], could you verify that?

[asfimport]

Konstantin Shaposhnikov / @kostya-sh:
@rdblue, you are right, I should have searched for an existing bug report first.

[asfimport]

Ryan Blue / @rdblue:
No problem, I just wanted to make sure.