[GitHub] [parquet-cpp] thaining opened pull request #495: PARQUET-1160: [C++] Implement BYTE_ARRAY-backed Decimal reads

["thaining (GitHub)" <gi...@apache.org>]

This change enables support for DECIMALs backed by BYTE_ARRAYs
on disk.  It does this by creating a TransferFunctor routine that
transforms a ByteArrayType to an ::arrow::Decimal128Type.

The routine does this by:
1. Creating an arrow::BinaryArray from the RecordReader's builder
2. Allocating a buffer for the arrow::Decimal128Array
3. Converting the big-endian bytes in each BinaryArray entry to two integers
   representing the high and low bits of each decimal value.

[ Full content available at: https://github.com/apache/parquet-cpp/pull/495 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org

[GitHub] [parquet-cpp] thaining commented on issue #495: PARQUET-1160: [C++] Implement BYTE_ARRAY-backed Decimal reads

["thaining (GitHub)" <gi...@apache.org>]

Fixed the indenting and column width changes in reader.cc suggested by 'make format' with clang-format 5.0.2.   For arrow-reader-writer-test.cc, the program only suggested reordering of the using statements, which I had not touched. 

[ Full content available at: https://github.com/apache/parquet-cpp/pull/495 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org

[GitHub] [parquet-cpp] majetideepak commented on pull request #495: PARQUET-1160: [C++] Implement BYTE_ARRAY-backed Decimal reads

["majetideepak (GitHub)" <gi...@apache.org>]

Nit: The statements in the two `if blocks` are the same. Should we combine them with an `||`?

[ Full content available at: https://github.com/apache/parquet-cpp/pull/495 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org

[GitHub] [parquet-cpp] wesm closed pull request #495: PARQUET-1160: [C++] Implement BYTE_ARRAY-backed Decimal reads

["wesm (GitHub)" <gi...@apache.org>]

[ pull request closed by wesm ]

[ Full content available at: https://github.com/apache/parquet-cpp/pull/495 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org

[GitHub] [parquet-cpp] thaining commented on issue #495: PARQUET-1160: [C++] Implement BYTE_ARRAY-backed Decimal reads

["thaining (GitHub)" <gi...@apache.org>]

> This PR needs to be moved to https://github.com/apache/arrow -- please let us know if you require assistance

Looks like I do require assistance.  The change squashes down to a clean commit, but I do not have write access to the repo.  Can you advise me on next steps?  Thanks.

Ted

[ Full content available at: https://github.com/apache/parquet-cpp/pull/495 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org

[GitHub] [parquet-cpp] thaining commented on issue #495: PARQUET-1160: [C++] Implement BYTE_ARRAY-backed Decimal reads

["thaining (GitHub)" <gi...@apache.org>]

Thanks for the help.   I was a little confused, and that gave me the push I needed.  I've created PR https://github.com/apache/arrow/pull/2646

[ Full content available at: https://github.com/apache/parquet-cpp/pull/495 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org

[GitHub] [parquet-cpp] wesm commented on issue #495: PARQUET-1160: [C++] Implement BYTE_ARRAY-backed Decimal reads

["wesm (GitHub)" <gi...@apache.org>]

I rolled up your changes as this commit https://github.com/wesm/arrow/commit/4f68fc3e46920f599ff6e5a0f37af5eb8cdc24e7. You can open a PR starting from there

[ Full content available at: https://github.com/apache/parquet-cpp/pull/495 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org

[GitHub] [parquet-cpp] thaining commented on issue #495: PARQUET-1160: [C++] Implement BYTE_ARRAY-backed Decimal reads

["thaining (GitHub)" <gi...@apache.org>]

@xhochy, a unit test conforming to the others for Spark was also included in the last batch of commits.

[ Full content available at: https://github.com/apache/parquet-cpp/pull/495 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org

[GitHub] [parquet-cpp] xhochy commented on issue #495: PARQUET-1160: [C++] Implement BYTE_ARRAY-backed Decimal reads

["xhochy (GitHub)" <gi...@apache.org>]

@thaining Travis reports some code style issues you could probably get rid of by running `make format`. Can you take care of that?

[ Full content available at: https://github.com/apache/parquet-cpp/pull/495 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org

[GitHub] [parquet-cpp] thaining commented on pull request #495: PARQUET-1160: [C++] Implement BYTE_ARRAY-backed Decimal reads

["thaining (GitHub)" <gi...@apache.org>]

I think I was being too literal in trying to model the structure of  TransferFunctor<::arrow::Decimal128Type, FLBAType>, since that has a for loop when (null_count > 0) and another where it doesn't. 

You are correct, an || is more compact.  I will update.  Thank you.

[ Full content available at: https://github.com/apache/parquet-cpp/pull/495 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org

[GitHub] [parquet-cpp] wesm commented on issue #495: PARQUET-1160: [C++] Implement BYTE_ARRAY-backed Decimal reads

["wesm (GitHub)" <gi...@apache.org>]

This PR needs to be moved to https://github.com/apache/arrow -- please let us know if you require assistance

[ Full content available at: https://github.com/apache/parquet-cpp/pull/495 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org