willtemperley commented on code in PR #95:
URL: https://github.com/apache/arrow-swift/pull/95#discussion_r2428169768
##########
Arrow/Sources/Arrow/ArrowWriter.swift:
##########
@@ -138,18 +138,26 @@ public class ArrowWriter { // swiftlint:disable:this
type_body_length
var rbBlocks = [org_apache_arrow_flatbuf_Block]()
for batch in batches {
+ addPadForAlignment(&writer)
let startIndex = writer.count
switch writeRecordBatch(batch: batch) {
case .success(let rbResult):
withUnsafeBytes(of: CONTINUATIONMARKER.littleEndian)
{writer.append(Data($0))}
withUnsafeBytes(of: rbResult.1.o.littleEndian)
{writer.append(Data($0))}
writer.append(rbResult.0)
+ addPadForAlignment(&writer)
Review Comment:
Side note - I can't see anything in the spec that says the
continuation+messageLength (8 bytes) is included in the metadata length, but
I'm fairly sure it does, doing a small experiment with PyArrow:
For example, with the example in `testFileWriter_bool`, if the block
metadata is written like this:
offset: 120
metadataLength: 208
bodyLength: 296
PyArrow throws an error:
pyarrow.lib.ArrowInvalid: flatbuffer size 8 invalid. File offset: 128,
metadata length: 208
However if the metadataLength includes the 8 byte prefix, i.e.:
offset: 120
metadataLength: 216
bodyLength: 296
The file is valid according to PyArrow.
--
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]
