[C#] Union array concatenation is incorrect #41198
Comments
CurtHagenlocher
pushed a commit
that referenced
this issue
Apr 16, 2024
### Rationale for this change Fixes concatenation of union arrays. ### What changes are included in this PR? * Re-enables union array concatenation tests that were disabled in #41197 after making union array comparisons more thorough in the `ArrowReaderVerifier` * Updates the union array concatenation logic to account for array lengths when concatenating the type and offset buffers, and fixes how the base offset is calculated. * Fixes creating the type buffers for the array concatenation tests. ### Are these changes tested? Yes, this uses the existing `ArrowArrayConcatenatorTests` tests. ### Are there any user-facing changes? Yes, this is a user-facing bug fix. * GitHub Issue: #41198 Authored-by: Adam Reeve <[email protected]> Signed-off-by: Curt Hagenlocher <[email protected]>
|
Issue resolved by pull request 41226 |
tolleybot
pushed a commit
to tmct/arrow
that referenced
this issue
May 2, 2024
### Rationale for this change Fixes concatenation of union arrays. ### What changes are included in this PR? * Re-enables union array concatenation tests that were disabled in apache#41197 after making union array comparisons more thorough in the `ArrowReaderVerifier` * Updates the union array concatenation logic to account for array lengths when concatenating the type and offset buffers, and fixes how the base offset is calculated. * Fixes creating the type buffers for the array concatenation tests. ### Are these changes tested? Yes, this uses the existing `ArrowArrayConcatenatorTests` tests. ### Are there any user-facing changes? Yes, this is a user-facing bug fix. * GitHub Issue: apache#41198 Authored-by: Adam Reeve <[email protected]> Signed-off-by: Curt Hagenlocher <[email protected]>
tolleybot
pushed a commit
to tmct/arrow
that referenced
this issue
May 4, 2024
### Rationale for this change Fixes concatenation of union arrays. ### What changes are included in this PR? * Re-enables union array concatenation tests that were disabled in apache#41197 after making union array comparisons more thorough in the `ArrowReaderVerifier` * Updates the union array concatenation logic to account for array lengths when concatenating the type and offset buffers, and fixes how the base offset is calculated. * Fixes creating the type buffers for the array concatenation tests. ### Are these changes tested? Yes, this uses the existing `ArrowArrayConcatenatorTests` tests. ### Are there any user-facing changes? Yes, this is a user-facing bug fix. * GitHub Issue: apache#41198 Authored-by: Adam Reeve <[email protected]> Signed-off-by: Curt Hagenlocher <[email protected]>
rok
pushed a commit
to tmct/arrow
that referenced
this issue
May 8, 2024
### Rationale for this change Fixes concatenation of union arrays. ### What changes are included in this PR? * Re-enables union array concatenation tests that were disabled in apache#41197 after making union array comparisons more thorough in the `ArrowReaderVerifier` * Updates the union array concatenation logic to account for array lengths when concatenating the type and offset buffers, and fixes how the base offset is calculated. * Fixes creating the type buffers for the array concatenation tests. ### Are these changes tested? Yes, this uses the existing `ArrowArrayConcatenatorTests` tests. ### Are there any user-facing changes? Yes, this is a user-facing bug fix. * GitHub Issue: apache#41198 Authored-by: Adam Reeve <[email protected]> Signed-off-by: Curt Hagenlocher <[email protected]>
rok
pushed a commit
to tmct/arrow
that referenced
this issue
May 8, 2024
### Rationale for this change Fixes concatenation of union arrays. ### What changes are included in this PR? * Re-enables union array concatenation tests that were disabled in apache#41197 after making union array comparisons more thorough in the `ArrowReaderVerifier` * Updates the union array concatenation logic to account for array lengths when concatenating the type and offset buffers, and fixes how the base offset is calculated. * Fixes creating the type buffers for the array concatenation tests. ### Are these changes tested? Yes, this uses the existing `ArrowArrayConcatenatorTests` tests. ### Are there any user-facing changes? Yes, this is a user-facing bug fix. * GitHub Issue: apache#41198 Authored-by: Adam Reeve <[email protected]> Signed-off-by: Curt Hagenlocher <[email protected]>
vibhatha
pushed a commit
to vibhatha/arrow
that referenced
this issue
May 25, 2024
### Rationale for this change Fixes concatenation of union arrays. ### What changes are included in this PR? * Re-enables union array concatenation tests that were disabled in apache#41197 after making union array comparisons more thorough in the `ArrowReaderVerifier` * Updates the union array concatenation logic to account for array lengths when concatenating the type and offset buffers, and fixes how the base offset is calculated. * Fixes creating the type buffers for the array concatenation tests. ### Are these changes tested? Yes, this uses the existing `ArrowArrayConcatenatorTests` tests. ### Are there any user-facing changes? Yes, this is a user-facing bug fix. * GitHub Issue: apache#41198 Authored-by: Adam Reeve <[email protected]> Signed-off-by: Curt Hagenlocher <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Describe the bug, including details regarding any error messages, version, and platform.
The
ArrayDataConcatenatordoes not currently handle union arrays correctly. It concatenates type and offset buffers without accounting for array lengths, and the way it calculates thebaseOffsetinConcatenateUnionOffsetBufferhas an off-by-one error and doesn't account for offsets being different per type.I've made union array comparison more thorough in #41197, which caused the array concatenation tests to start failing, so I've temporarily skipped testing of union array concatenation in that PR.
Component(s)
C#
The text was updated successfully, but these errors were encountered: