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.

Sign up for GitHub

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

Jump to bottom

[C#] Union arrays don't account for offset and length #41140

Closed
adamreeve opened this issue Apr 11, 2024 · 2 comments
Closed

[C#] Union arrays don't account for offset and length #41140

adamreeve opened this issue Apr 11, 2024 · 2 comments
Assignees
@adamreeve
Labels
Component: C# Type: bug
Milestone
16.1.0

Comments

@adamreeve
Copy link
Contributor

Describe the bug, including details regarding any error messages, version, and platform.

Similar to #40790, a sliced union array doesn't behave as expected as none of the methods or properties account for the offset and length.

This should work differently for sparse and dense unions. For a sparse union, the child arrays should be sliced when accessed, and the offset and slice should be accounted for in the TypeIds span. For a dense union, the child arrays shouldn't be sliced but the TypeIds and ValueOffsets spans should.

Component(s)

C#
@adamreeve adamreeve mentioned this issue Apr 12, 2024
Merged
CurtHagenlocher pushed a commit that referenced this issue Apr 12, 2024
@adamreeve
GH-41140: [C#] Account for offset and length in union arrays (#41165)
48a9639 
### Rationale for this change

See #41140. This makes a sliced union array behave as expected without having to manually account for the array offset unless accessing the underlying buffers.

### What changes are included in this PR?

Accounts for the offset and length when getting type ids, value offsets and field arrays for sparse and dense union arrays.

### Are these changes tested?

Yes, I've updated the union array tests to cover this.

### Are there any user-facing changes?

Yes, this is a user facing bug fix.
* GitHub Issue: #41140

Authored-by: Adam Reeve <[email protected]>
Signed-off-by: Curt Hagenlocher <[email protected]>
@CurtHagenlocher CurtHagenlocher added this to the 17.0.0 milestone Apr 12, 2024
@CurtHagenlocher
Copy link
Contributor

Issue resolved by pull request 41165
#41165

vibhatha pushed a commit to vibhatha/arrow that referenced this issue Apr 15, 2024
@adamreeve @vibhatha
apacheGH-41140: [C#] Account for offset and length in union arrays (a…
de65351 
…pache#41165)

### Rationale for this change

See apache#41140. This makes a sliced union array behave as expected without having to manually account for the array offset unless accessing the underlying buffers.

### What changes are included in this PR?

Accounts for the offset and length when getting type ids, value offsets and field arrays for sparse and dense union arrays.

### Are these changes tested?

Yes, I've updated the union array tests to cover this.

### Are there any user-facing changes?

Yes, this is a user facing bug fix.
* GitHub Issue: apache#41140

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 2, 2024
@adamreeve @tolleybot
apacheGH-41140: [C#] Account for offset and length in union arrays (a…
8ba5d7b 
…pache#41165)

### Rationale for this change

See apache#41140. This makes a sliced union array behave as expected without having to manually account for the array offset unless accessing the underlying buffers.

### What changes are included in this PR?

Accounts for the offset and length when getting type ids, value offsets and field arrays for sparse and dense union arrays.

### Are these changes tested?

Yes, I've updated the union array tests to cover this.

### Are there any user-facing changes?

Yes, this is a user facing bug fix.
* GitHub Issue: apache#41140

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
@adamreeve @tolleybot
apacheGH-41140: [C#] Account for offset and length in union arrays (a…
acd4652 
…pache#41165)

### Rationale for this change

See apache#41140. This makes a sliced union array behave as expected without having to manually account for the array offset unless accessing the underlying buffers.

### What changes are included in this PR?

Accounts for the offset and length when getting type ids, value offsets and field arrays for sparse and dense union arrays.

### Are these changes tested?

Yes, I've updated the union array tests to cover this.

### Are there any user-facing changes?

Yes, this is a user facing bug fix.
* GitHub Issue: apache#41140

Authored-by: Adam Reeve <[email protected]>
Signed-off-by: Curt Hagenlocher <[email protected]>
@raulcd raulcd modified the milestones: 17.0.0, 16.1.0 May 8, 2024
@raulcd
Copy link
Member

raulcd commented May 8, 2024

Adding this to 16.1.0 as this seems to fix the tests issue locally for me: #41397

raulcd pushed a commit that referenced this issue May 8, 2024
@adamreeve @raulcd
GH-41140: [C#] Account for offset and length in union arrays (#41165)
dcfeceb 
### Rationale for this change

See #41140. This makes a sliced union array behave as expected without having to manually account for the array offset unless accessing the underlying buffers.

### What changes are included in this PR?

Accounts for the offset and length when getting type ids, value offsets and field arrays for sparse and dense union arrays.

### Are these changes tested?

Yes, I've updated the union array tests to cover this.

### Are there any user-facing changes?

Yes, this is a user facing bug fix.
* GitHub Issue: #41140

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
@adamreeve @rok
apacheGH-41140: [C#] Account for offset and length in union arrays (a…
088d658 
…pache#41165)

### Rationale for this change

See apache#41140. This makes a sliced union array behave as expected without having to manually account for the array offset unless accessing the underlying buffers.

### What changes are included in this PR?

Accounts for the offset and length when getting type ids, value offsets and field arrays for sparse and dense union arrays.

### Are these changes tested?

Yes, I've updated the union array tests to cover this.

### Are there any user-facing changes?

Yes, this is a user facing bug fix.
* GitHub Issue: apache#41140

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
@adamreeve @rok
apacheGH-41140: [C#] Account for offset and length in union arrays (a…
f01a957 
…pache#41165)

### Rationale for this change

See apache#41140. This makes a sliced union array behave as expected without having to manually account for the array offset unless accessing the underlying buffers.

### What changes are included in this PR?

Accounts for the offset and length when getting type ids, value offsets and field arrays for sparse and dense union arrays.

### Are these changes tested?

Yes, I've updated the union array tests to cover this.

### Are there any user-facing changes?

Yes, this is a user facing bug fix.
* GitHub Issue: apache#41140

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
@adamreeve @vibhatha
apacheGH-41140: [C#] Account for offset and length in union arrays (a…
e3fae99 
…pache#41165)

### Rationale for this change

See apache#41140. This makes a sliced union array behave as expected without having to manually account for the array offset unless accessing the underlying buffers.

### What changes are included in this PR?

Accounts for the offset and length when getting type ids, value offsets and field arrays for sparse and dense union arrays.

### Are these changes tested?

Yes, I've updated the union array tests to cover this.

### Are there any user-facing changes?

Yes, this is a user facing bug fix.
* GitHub Issue: apache#41140

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
Assignees

@adamreeve adamreeve

Labels
Component: C# Type: bug
Projects
None yet
Milestone
16.1.0
Development

No branches or pull requests
3 participants
@adamreeve @raulcd @CurtHagenlocher