Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ARROW-8467: [C++] Fix Array::View tests for big-endian platforms #6944

Closed
wants to merge 1 commit into from

Conversation

kiszk
Copy link
Member

@kiszk kiszk commented Apr 15, 2020

This PR adds the appropriate input on big-endian platform for test cases using ArrayFromJSON with fixed_size_binary.

I saw a problem with the following test:

TEST(TestArrayView, PrimitiveAsFixedSizeBinary) {
  auto arr = ArrayFromJSON(int32(), "[2020568934, 2054316386, null]");
  auto expected = ArrayFromJSON(fixed_size_binary(4), R"(["foox", "barz", null])");
  CheckView(arr, expected);
  CheckView(expected, arr);
}

Here, the expected strings are represented in binary as follows:

"foox" = [0x66 0x6f 0x6f 0x78]
"barz" = [0x62 0x61 0x72 0x7a]

This test gives a value as raw int32. The current values assume only a little-endian platform as follows to generate the expected binary sequence.

2020568934 = 0x786f6f66
2054316386 = 0x7a726162

IMHO, for big-endian platform, the following value should be given

1718579064 = 0x666f6f78
1650553466 = 0x6261727a

These tests have the above problems:
TEST_F(TestChunkedArray, View) at https://github.com/apache/arrow/blob/master/cpp/src/arrow/table_test.cc#L175
TEST(TestArrayView, PrimitiveAsFixedSizeBinary) at https://github.com/apache/arrow/blob/master/cpp/src/arrow/array_view_test.cc#L105
TEST(TestArrayView, StructAsStructSimple) at https://github.com/apache/arrow/blob/master/cpp/src/arrow/array_view_test.cc#L126
TEST(TestArrayView, ExtensionType) at https://github.com/apache/arrow/blob/master/cpp/src/arrow/array_view_test.cc#L393

@github-actions
Copy link

@pitrou pitrou changed the title ARROW-8467: [C++] Tests using ArrayFromJSON with fixed_size_binary supports big-endian platform ARROW-8467: [C++] Fix Array::View tests for big-endian platforms Apr 15, 2020
Copy link
Member

@pitrou pitrou left a comment

Choose a reason for hiding this comment

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

+1, thank you

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.

None yet

2 participants