Handle int64 strings offsets in cudf::byte_cast API by davidwendt · Pull Request #23725 · NVIDIA/cudf

davidwendt · 2026-08-19T17:45:34Z

Description

Fixes logic in the cudf::byte_cast API which converts a strings column into a list column and properly handles int64 offsets by casting them to INT32 if possible.

Reference #23685 (review)

Checklist

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

copy-pr-bot · 2026-08-19T17:45:38Z

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

davidwendt · 2026-08-19T17:46:02Z

@coderabbitai full review

davidwendt · 2026-08-19T17:46:15Z

/ok to test

coderabbitai · 2026-08-19T17:46:17Z

✅ Action performed

Full review finished.

coderabbitai · 2026-08-19T17:48:30Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: e8ffd3dd-9661-4a4d-bbbd-abd00963d285

📥 Commits

Reviewing files that changed from the base of the PR and between 1a4b9b6 and 399f50e.

📒 Files selected for processing (1)

cpp/src/reshape/byte_cast.cu

🚧 Files skipped from review as they are similar to previous changes (1)

cpp/src/reshape/byte_cast.cu

Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review.

📝 Walkthrough

Summary by CodeRabbit

Bug Fixes
- Improved string byte-casting reliability by ensuring offsets use the correct 32-bit integer format.
- Added compatibility handling for inputs with different offset types.
- Added safeguards to restrict string byte conversion to supported 32-bit offset limits, helping prevent invalid results for oversized inputs.

Walkthrough

The string byte-casting path enforces INT32 character-data limits and normalizes string offsets to INT32 before constructing the lists column. The change also adds the unary utility header.

Changes

String byte-casting

Layer / File(s)	Summary
Normalize offsets before list construction `cpp/src/reshape/byte_cast.cu`	The path rejects character data at or above the `INT32` maximum, extracts the offsets child, and casts it to `INT32` when needed before calling `make_lists_column`. It adds the unary utility header.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 399f5

This localized fix changes int64 offset handling in the byte-cast path; no actionable merge-blocking risk remains beyond normal checks and review.

Suggested reviewers: mattgara, qbacpey

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description check	✅ Passed	The description clearly explains the changes to cudf::byte_cast and the handling of int64 string offsets.
Title check	✅ Passed	The title clearly identifies the main change: handling int64 string offsets in the cudf::byte_cast API.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

_{Comment @coderabbitai help to get the list of available commands.}

wence- · 2026-08-25T13:10:10Z

-      detail::copy_bitmask(input, stream, mr));
+    auto offsets_col = std::move(col_content.children[strings_column_view::offsets_column_index]);
+    if (offsets_col->type().id() != type_id::INT32) {
+      offsets_col = cudf::detail::cast(offsets_col->view(), data_type{type_id::INT32}, stream, mr);


question: What happens if the string offsets are too large to cast into INT32?

I believe the check on line 125 ensures all the offsets values are less than max(int32).
Probably should change max(size_type) to max(int32) there until large lists are supported.

wence-

I can see that this is the right thing for int64 string offsets if all of those fit in int32, but what if they don't?

wence-

Optional: change overflow check on line 125 to std::numeric_limits<int32_t>::max()

coderabbitai · 2026-08-27T19:42:45Z

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

davidwendt · 2026-08-28T14:31:23Z

/merge

Handle large strings offsets in cudf::byte_cast API

1a3593a

davidwendt self-assigned this Aug 19, 2026

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 Aug 19, 2026

davidwendt added 2 commits August 20, 2026 11:39

Merge branch 'main' into ls-byte-cast

d316170

Merge branch 'main' into ls-byte-cast

c9ec30f

davidwendt added 3 - Ready for Review Ready for review by team and removed 2 - In Progress Currently a work in progress labels Aug 21, 2026

davidwendt marked this pull request as ready for review August 21, 2026 14:43

davidwendt requested a review from a team as a code owner August 21, 2026 14:43

davidwendt requested review from mattgara and qbacpey August 21, 2026 14:43

Merge branch 'main' into ls-byte-cast

320226c

wence- approved these changes Aug 25, 2026

View reviewed changes

wence- reviewed Aug 25, 2026

View reviewed changes

wence- requested changes Aug 25, 2026

View reviewed changes

bdice approved these changes Aug 25, 2026

View reviewed changes

davidwendt added 2 commits August 26, 2026 09:39

Merge branch 'main' into ls-byte-cast

45a45f2

Merge branch 'main' into ls-byte-cast

5f6478c

davidwendt requested a review from wence- August 27, 2026 13:03

wence- approved these changes Aug 27, 2026

View reviewed changes

davidwendt added 2 commits August 27, 2026 11:23

Merge branch 'main' into ls-byte-cast

b1c4ba0

size_type to int32_t

bf9d22c

Merge branch 'main' into ls-byte-cast

399f50e

rapids-bot Bot merged commit 6f902ce into NVIDIA:main Aug 28, 2026
158 checks passed

davidwendt deleted the ls-byte-cast branch August 28, 2026 14:31

Uh oh!

Conversation

davidwendt commented Aug 19, 2026

Description

Checklist

Uh oh!

copy-pr-bot Bot commented Aug 19, 2026

Uh oh!

davidwendt commented Aug 19, 2026

Uh oh!

davidwendt commented Aug 19, 2026

Uh oh!

coderabbitai Bot commented Aug 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot commented Aug 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

wence- Aug 25, 2026

Choose a reason for hiding this comment

Uh oh!

davidwendt Aug 25, 2026

Choose a reason for hiding this comment

Uh oh!

wence- Aug 27, 2026

Choose a reason for hiding this comment

Uh oh!

wence- left a comment

Choose a reason for hiding this comment

Uh oh!

wence- left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot commented Aug 27, 2026

Uh oh!

davidwendt commented Aug 28, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

coderabbitai Bot commented Aug 19, 2026 •

edited

Loading

coderabbitai Bot commented Aug 19, 2026 •

edited

Loading