jding-xyz commented on PR #152: URL: https://github.com/apache/arrow-swift/pull/152#issuecomment-4151966090
> Comments suppressed due to low confidence (1) > **Sources/Arrow/ArrowCImporter.swift:144** > > * In the C data importer, `appendToBuffer` sets `ArrowBuffer.capacity` equal to the passed `length`, but `capacity` is treated as a byte count elsewhere (e.g., IPC writer uses `buffer.capacity` for buffer sizes). Passing `length + 1` for the offsets buffer makes `capacity` represent an element count, not bytes, which can lead to incorrect serialization sizes for imported arrays. Consider updating `appendToBuffer` to accept an element size (or explicit byteLength) so the offsets buffer uses `(length + 1) * MemoryLayout<Int32>.stride` bytes for capacity (while keeping `length` as the element count). > > ``` > appendToBuffer(cArray.buffers[0], arrowBuffers: &arrowBuffers, length: (length + 7) / 8) > appendToBuffer(cArray.buffers[1], arrowBuffers: &arrowBuffers, length: length + 1) > let lastOffsetLength = cArray.buffers[1]! > .advanced(by: Int(length) * MemoryLayout<Int32>.stride) > .load(as: Int32.self) > appendToBuffer(cArray.buffers[2], arrowBuffers: &arrowBuffers, length: UInt(lastOffsetLength)) > ``` > > 💡 [Add Copilot custom instructions](/apache/arrow-swift/new/main?filename=.github/instructions/*.instructions.md) for smarter, more guided reviews. [Learn how to get started](https://docs.github.com/en/copilot/customizing-copilot/adding-repository-custom-instructions-for-github-copilot). I think this is a real issue but it needs more consideration and out of scope of this PR. We should address it later. -- 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]
