Jay846 commented on PR #50976: URL: https://github.com/apache/arrow/pull/50976#issuecomment-5666955466
Hi @pitrou, @HuaHuaY, Here is a full summary of all review feedback addressed in the PR: 1. **Kernel Renaming & Template Documentation:** Renamed `CastListView` to `CastListViewToVarList` and added template documentation `// (Large)ListView<T> -> (Large)List<U>`. 2. **Pre-condition Safeguard:** Replaced `in_array.length == 0` runtime branching with `DCHECK_NE(in_array.length, 0)`. 3. **Offset Allocation:** Factored out `dest_offsets` buffer allocation above the `if/else` block. 4. **Clean End Offset Calculation:** Simplified `end_offset` to use absolute offset (`abs_end_offset`) without redundant `start_offset` subtraction. 5. **SetBitRunReader Bitmap Traversal:** Upgraded the non-contiguous path to use `arrow::internal::SetBitRunReader` for word-at-a-time validity scanning, including a fast-path when `validity == nullptr` (`c929f7e`). 6. **Linter & Formatting Compliance:** Applied `clang-format` line wrapping and formatting rules (`b312f28`). `Dev / Lint` CI is 100% green. 7. **Test Coverage:** Added `ValidateFull()` checks, null-heavy contiguous examples, and zero-length array test cases for both `list_view` and `large_list_view`. All unit tests pass cleanly. The PR is fully up to date. Thanks! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
