You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2020/05/24 06:12:15 UTC
[GitHub] [arrow] kiszk opened a new pull request #7256: ARROW-8914: [C++][Gandiva] Keep BasicDecimal128 in native-endian order
kiszk opened a new pull request #7256:
URL: https://github.com/apache/arrow/pull/7256
This PR changes to keep BasicDecimal128 in native-endian order instead of little-endian order.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] fsaintjacques closed pull request #7256: ARROW-8914: [C++] Keep BasicDecimal128 in native-endian order
Posted by GitBox <gi...@apache.org>.
fsaintjacques closed pull request #7256:
URL: https://github.com/apache/arrow/pull/7256
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #7256: ARROW-8914: [C++][Gandiva] Keep BasicDecimal128 in native-endian order
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #7256:
URL: https://github.com/apache/arrow/pull/7256#issuecomment-633185508
https://issues.apache.org/jira/browse/ARROW-8914
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] fsaintjacques commented on a change in pull request #7256: ARROW-8914: [C++][Gandiva] Keep BasicDecimal128 in native-endian order
Posted by GitBox <gi...@apache.org>.
fsaintjacques commented on a change in pull request #7256:
URL: https://github.com/apache/arrow/pull/7256#discussion_r432488488
##########
File path: cpp/src/arrow/util/basic_decimal.cc
##########
@@ -139,8 +144,13 @@ std::array<uint8_t, 16> BasicDecimal128::ToBytes() const {
void BasicDecimal128::ToBytes(uint8_t* out) const {
DCHECK_NE(out, nullptr);
- reinterpret_cast<uint64_t*>(out)[0] = BitUtil::ToLittleEndian(low_bits_);
- reinterpret_cast<int64_t*>(out)[1] = BitUtil::ToLittleEndian(high_bits_);
+#if ARROW_LITTLE_ENDIAN
+ reinterpret_cast<uint64_t*>(out)[0] = low_bits_;
Review comment:
You could replace this with the SafeLoad/SafeStore functions.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kiszk commented on pull request #7256: ARROW-8914: [C++][Gandiva] Keep BasicDecimal128 in native-endian order
Posted by GitBox <gi...@apache.org>.
kiszk commented on pull request #7256:
URL: https://github.com/apache/arrow/pull/7256#issuecomment-633188543
Now, only one test suite fails as https://travis-ci.org/github/apache/arrow/jobs/690545840#L1805
```
99% tests passed, 1 tests failed out of 76
Label Time Summary:
arrow-tests = 16.33 sec*proc (28 tests)
arrow_compute = 11.46 sec*proc (14 tests)
arrow_dataset = 1.21 sec*proc (8 tests)
arrow_flight = 0.28 sec*proc (1 test)
arrow_ipc = 3.00 sec*proc (3 tests)
gandiva-tests = 12.44 sec*proc (19 tests)
plasma-tests = 11.35 sec*proc (3 tests)
unittest = 56.07 sec*proc (76 tests)
Total Test time (real) = 15.49 sec
The following tests FAILED:
67 - gandiva-date-time-test (Failed)
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] fsaintjacques commented on a change in pull request #7256: ARROW-8914: [C++] Keep BasicDecimal128 in native-endian order
Posted by GitBox <gi...@apache.org>.
fsaintjacques commented on a change in pull request #7256:
URL: https://github.com/apache/arrow/pull/7256#discussion_r432488488
##########
File path: cpp/src/arrow/util/basic_decimal.cc
##########
@@ -139,8 +144,13 @@ std::array<uint8_t, 16> BasicDecimal128::ToBytes() const {
void BasicDecimal128::ToBytes(uint8_t* out) const {
DCHECK_NE(out, nullptr);
- reinterpret_cast<uint64_t*>(out)[0] = BitUtil::ToLittleEndian(low_bits_);
- reinterpret_cast<int64_t*>(out)[1] = BitUtil::ToLittleEndian(high_bits_);
+#if ARROW_LITTLE_ENDIAN
+ reinterpret_cast<uint64_t*>(out)[0] = low_bits_;
Review comment:
You could replace this with the SafeLoad/SafeStore functions.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org