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

Add single offset to an empty ListArray in cudf::to_arrow #16201

Merged
merged 1 commit into from
Jul 8, 2024

Conversation

davidwendt
Copy link
Contributor

Description

Closes #16164

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@davidwendt davidwendt added bug Something isn't working 2 - In Progress Currently a work in progress libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change labels Jul 5, 2024
@davidwendt davidwendt self-assigned this Jul 5, 2024
@davidwendt
Copy link
Contributor Author

@wence- Just checking if this is compatible with #16198

@wence-
Copy link
Contributor

wence- commented Jul 5, 2024

I think so, yes

@davidwendt davidwendt added 3 - Ready for Review Ready for review by team and removed 2 - In Progress Currently a work in progress labels Jul 5, 2024
@davidwendt davidwendt changed the title Add zero offset to ListArray in cudf::to_arrow Add single offset to an empty ListArray in cudf::to_arrow Jul 5, 2024
@davidwendt davidwendt marked this pull request as ready for review July 5, 2024 16:47
@davidwendt davidwendt requested a review from a team as a code owner July 5, 2024 16:47
@davidwendt
Copy link
Contributor Author

/merge

@rapids-bot rapids-bot bot merged commit 2664427 into rapidsai:branch-24.08 Jul 8, 2024
81 checks passed
@davidwendt davidwendt deleted the empty-list-to-arrow branch July 8, 2024 18:00
rapids-bot bot pushed a commit that referenced this pull request Jul 16, 2024
An empty list column need not have empty children, it just needs to have zero length. In this case, the offsets array will have zero length, and we need to create a temporary buffer.

Now that this branch runs, fix two errors in the construction of the arrow array:

1. The element type, if there are children, should be taken from the child array;
2. If the child arrays are empty, we must make an empty null array, rather than passing a null pointer as the values array, otherwise we hit a segfault inside arrow.

The previous fix in #16201 correctly handled the empty children case (except for point two), but not the first case, which we do here.

Since we we're previously going down this code path (child_arrays was never empty), we never hit the latent segfault from point two.

Authors:
  - Lawrence Mitchell (https://github.com/wence-)

Approvers:
  - David Wendt (https://github.com/davidwendt)
  - MithunR (https://github.com/mythrocks)

URL: #16279
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 - Ready for Review Ready for review by team bug Something isn't working libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] make_lists_column does not enforce arrow-compatible invariants on offsets column
5 participants