GH-48194: [C++] Fix arrow-bit-utility-test failure on s390x

[Vishwanatha-HD]

Rationale for this change

This PR is intended to enable Parquet DB support on Big-endian (s390x) systems. The fix in this PR fixes "arrow-bit-utility-test" testcase failure.

The "arrow-bit-utility-test" testcase was failing with 1 Test case Failure on Big-endian platforms.

[ FAILED ] 1 test, listed below:
[ FAILED ] BitStreamUtil.ZigZag

What changes are included in this PR?

The fix includes changes to "bit_stream_utils_internal.h" file to address the Abort/Core dump issues.

Are these changes tested?

Yes. The changes are tested on s390x arch to make sure things are working fine. The fix is also tested on x86 arch, to make sure there is no new regression introduced.

Are there any user-facing changes?

No

• GitHub main Issue link: [C++][Parquet] Enable Parquet DB support on Big Endian (IBM Z) systems #48151

• GitHub Issue: [C++] arrow-bit-utility-test failed on Big-Endian (s390x) systems #48194

[github-actions]

⚠️ GitHub issue #48194 has been automatically assigned in GitHub to PR creator.

[kou]

Suggested change

	// with buffered_values_ on big-endian systems like s390x
	// with buffered_values_ on big-endian systems like s390x.

[kou]

Suggested change

	// Calculate current position in buffer accounting for bit offset
	// Calculate current position in buffer accounting for bit offset.

[kou]

Suggested change

	// The data that we will pass to the LEB128 parser
	// The data that we will pass to the LEB128 parser.

[kou]

Suggested change

	// In all case, we read a byte-aligned value, skipping remaining bits
	// In all case, we read a byte-aligned value, skipping remaining bits.

[kou]

Could you revert this change?
(This is needless, right?)

[Vishwanatha-HD]

Hi @kou.. Yes certainly, this is needless.. I had earlier tried "git restore" and "git reset --hard" to remove "testing" changes.. But I didnt realize that this is a sub-module and I need to do "git submodule update --init --recursive"...
Now, I have dont that and removed the changes.. thanks..

[kou]

Does this the same logic as

arrow/cpp/src/arrow/util/bit_stream_utils_internal.h

Lines 380 to 383 in 2fb2f79

	} else {
	max_size = bytes_left();
	data = buffer_ + (max_bytes_ - max_size);
	}

?

If so, should we reuse it something like the following?

diff --git a/cpp/src/arrow/util/bit_stream_utils_internal.h b/cpp/src/arrow/util/bit_stream_utils_internal.h
index d8c7317fe8..7352312782 100644
--- a/cpp/src/arrow/util/bit_stream_utils_internal.h
+++ b/cpp/src/arrow/util/bit_stream_utils_internal.h
@@ -366,6 +366,9 @@ inline bool BitReader::GetVlqInt(Int* v) {
   const uint8_t* data = NULLPTR;
   int max_size = 0;
 
+#if ARROW_LITTLE_ENDIAN
+  // TODO: Describe why we need this only for little-endian.
+
   // Number of bytes left in the buffered values, not including the current
   // byte (i.e., there may be an additional fraction of a byte).
   const int bytes_left_in_cache =
@@ -377,7 +380,9 @@ inline bool BitReader::GetVlqInt(Int* v) {
     data = reinterpret_cast<const uint8_t*>(&buffered_values_) +
            bit_util::BytesForBits(bit_offset_);
     // Otherwise, we try straight from buffer (ignoring few bytes that may be cached)
-  } else {
+  } else
+#endif
+  {
     max_size = bytes_left();
     data = buffer_ + (max_bytes_ - max_size);
   }