zanmato1984 commented on code in PR #46706:
URL: https://github.com/apache/arrow/pull/46706#discussion_r2132867331
##########
cpp/src/arrow/ipc/writer.cc:
##########
@@ -329,15 +329,24 @@ class RecordBatchSerializer {
return Status::OK();
}
- int64_t required_bytes = sizeof(offset_type) * (array.length() + 1);
- if (array.value_offset(0) > 0) {
+ const int64_t required_bytes = sizeof(offset_type) * (array.length() + 1);
+
+ offset_type first_offset = 0;
+ RETURN_NOT_OK(MemoryManager::CopyBufferSliceToCPU(
Review Comment:
I think I'll need some context here. Is this function supposed to be able to
deal with non-CPU arrays? Does this rule apply to other functions in this
source file too?
##########
cpp/src/arrow/ipc/writer.cc:
##########
@@ -329,15 +329,24 @@ class RecordBatchSerializer {
return Status::OK();
}
- int64_t required_bytes = sizeof(offset_type) * (array.length() + 1);
- if (array.value_offset(0) > 0) {
+ const int64_t required_bytes = sizeof(offset_type) * (array.length() + 1);
+
+ offset_type first_offset = 0;
+ RETURN_NOT_OK(MemoryManager::CopyBufferSliceToCPU(
+ array.data()->buffers[1], array.offset() * sizeof(offset_type),
+ sizeof(offset_type), reinterpret_cast<uint8_t*>(&first_offset)));
+
+ if (first_offset > 0) {
// If the offset of the first value is non-zero, then we must create a
new
// offsets buffer with shifted offsets.
+ if (!array.data()->buffers[1]->is_cpu()) {
+ return Status::NotImplemented("Rebasing non-CPU offsets");
+ }
Review Comment:
Two questions:
1. Is this error handling necessary to fix the "CUDA tests" mentioned in the
PR description?
2. It appears that if the CUDA tests were failing with accessing the non-CPU
`first_offset`, and the above `MemoryManager::CopyBufferSliceToCPU` is supposed
to fix it, wouldn't this check fail the tests again with `Unimplemented`?
--
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]
