Skip to content

Conversation

@thaining
Copy link
Contributor

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.

…k. 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.
@wesm wesm force-pushed the parquet-1160-byte-array-decimals branch from c13f857 to 30f3a27 Compare September 28, 2018 09:53
@wesm
Copy link
Member

wesm commented Sep 28, 2018

rebased

@xhochy
Copy link
Member

xhochy commented Sep 29, 2018

@thaining ReadDecimals/TestArrowReaderAdHocSparkAndHvr.ReadDecimals/4 is failing in CI, can you have a look what's going wrong?

@thaining
Copy link
Contributor Author

@xhochy, I am able to reproduce the problem locally by running 'ctest -L parquet'. The problem is that the SHA for cpp/submodules/parquet-testing does not have the file byte_array_decimal.parquet needed for the test. If I manually add that parquet file to $ARROW_ROOT/cpp/submodules/parquet-testing/data, the test passes.

When I do "git submodule update --init" on the arrow tree, it leaves the submodule at "HEAD detached at 48a657c". That's the wrong SHA. If I switch the submodule to the latest trunk, I get SHA 46ae260, which has the file. The test also passes.

Copy link
Member

@xhochy xhochy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1, LGTM

Build failure is unrelated (wheel/multibuild incompability)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants