westonpace commented on code in PR #35565:
URL: https://github.com/apache/arrow/pull/35565#discussion_r1205216984
##########
cpp/src/arrow/util/align_util.cc:
##########
@@ -94,12 +135,17 @@ bool CheckAlignment(const Table& table, int64_t alignment,
return all_aligned;
}
+// Most allocators require a minimum of 8-byte alignment.
+constexpr int64_t kMinimumAlignment = 8;
Review Comment:
The problem arises when we are checking for value alignment. For example,
if we encounter a buffer that requires 4 byte alignment then we will call:
```
EnsureAlignment(buffer, 4, memory_pool);
```
If that buffer was not 4-byte aligned then we would try and allocate a new
buffer:
```
ARROW_ASSIGN_OR_RAISE(
auto new_buffer,
AllocateBuffer(buffer->size(), 4, memory_pool));
```
This would then raise.
Another option would be to change the signature of `EnsureAlignment`:
```
Result<std::shared_ptr<Buffer>> EnsureAlignment(std::shared_ptr<Buffer>
buffer,
int64_t expected_alignment,
int64_t
new_alignment_if_needed,
MemoryPool* memory_pool)
```
Then we could pass `4` for `expected_alignment` and
`kDefaultBufferAlignment` for `new_alignment_if_needed`. I wasn't sure if it
was worth cluttering the signature of `EnsureAlignment` or not.
--
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]
