From c584c7e0dc70617d83c23eedb183b0554d464b43 Mon Sep 17 00:00:00 2001 From: Dewey Dunnington Date: Fri, 19 Apr 2024 10:07:28 -0300 Subject: [PATCH] GH-41263: [C#][Integration] Ensure offset is considered in all branches of the bitmap comparison (#41264) ### Rationale for this change The optimization for validity buffers was still failing after https://github.com/apache/arrow/pull/41259 (sorry!). ### What changes are included in this PR? There were still two problems: - The offset of the actual array was not considered in the "optimized" branch - When this offset *was* considered, it became clear that zero-length arrays were not going to work in that branch ### Are these changes tested? I added the integration workflow to also run for C# additions. This might be a heavy CI job and I'm not sure if you want to keep it there (but running it is useful for this PR to ensure I actually fix things). For future me (or maybe future others), the integration tests are pretty easy to check: ``` dotnet build archery integration --run-c-data --with-csharp=true ``` ### Are there any user-facing changes? No. * GitHub Issue: #41263 Authored-by: Dewey Dunnington Signed-off-by: Curt Hagenlocher --- .github/workflows/integration.yml | 2 ++ .../Apache.Arrow.Tests/ArrowReaderVerifier.cs | 27 ++----------------- 2 files changed, 4 insertions(+), 25 deletions(-) diff --git a/.github/workflows/integration.yml b/.github/workflows/integration.yml index 2c3499c160f9c..6e09ad61480a6 100644 --- a/.github/workflows/integration.yml +++ b/.github/workflows/integration.yml @@ -29,6 +29,7 @@ on: - 'js/**' - 'cpp/**' - 'java/**' + - 'csharp/**' - 'format/**' pull_request: paths: @@ -40,6 +41,7 @@ on: - 'integration/**' - 'js/**' - 'cpp/**' + - 'csharp/**' - 'java/**' - 'format/**' diff --git a/csharp/test/Apache.Arrow.Tests/ArrowReaderVerifier.cs b/csharp/test/Apache.Arrow.Tests/ArrowReaderVerifier.cs index 360c3d71296c7..5c33d1fd43986 100644 --- a/csharp/test/Apache.Arrow.Tests/ArrowReaderVerifier.cs +++ b/csharp/test/Apache.Arrow.Tests/ArrowReaderVerifier.cs @@ -497,36 +497,13 @@ private void CompareValidityBuffer(int nullCount, int arrayLength, ArrowBuffer e { Assert.True(expectedValidityBuffer.Span.SequenceEqual(actualValidityBuffer.Span)); } - else if (actualValidityBuffer.IsEmpty) + else if (actualValidityBuffer.IsEmpty || expectedValidityBuffer.IsEmpty || arrayLength == 0) { Assert.True(nullCount == 0 || arrayLength == 0); } - else if (expectedBufferOffset % 8 == 0 && expectedBufferOffset == actualBufferOffset) - { - int validityBitmapByteCount = BitUtility.ByteCount(arrayLength); - int byteOffset = BitUtility.ByteCount(expectedBufferOffset); - ReadOnlySpan expectedSpanPartial = expectedValidityBuffer.Span.Slice(byteOffset, validityBitmapByteCount - 1); - ReadOnlySpan actualSpanPartial = actualValidityBuffer.Span.Slice(0, validityBitmapByteCount - 1); - - // Compare the first validityBitmapByteCount - 1 bytes - Assert.True( - expectedSpanPartial.SequenceEqual(actualSpanPartial), - string.Format("First {0} bytes of validity buffer do not match", validityBitmapByteCount - 1)); - - // Compare the last byte bitwise (because there is no guarantee about the value of - // bits outside the range [0, arrayLength]) - ReadOnlySpan expectedSpanFull = expectedValidityBuffer.Span.Slice(byteOffset, validityBitmapByteCount); - ReadOnlySpan actualSpanFull = actualValidityBuffer.Span.Slice(0, validityBitmapByteCount); - for (int i = 8 * (validityBitmapByteCount - 1); i < arrayLength; i++) - { - Assert.True( - BitUtility.GetBit(expectedSpanFull, i) == BitUtility.GetBit(actualSpanFull, i), - string.Format("Bit at index {0}/{1} is not equal", i, arrayLength)); - } - } else { - // Have to compare all values bitwise + // Compare all values bitwise var expectedSpan = expectedValidityBuffer.Span; var actualSpan = actualValidityBuffer.Span; for (int i = 0; i < arrayLength; i++)