kou commented on code in PR #45029:
URL: https://github.com/apache/arrow/pull/45029#discussion_r1885453221


##########
swift/Arrow/Sources/Arrow/ArrowReader.swift:
##########
@@ -216,7 +216,67 @@ public class ArrowReader { // swiftlint:disable:this 
type_body_length
         return .success(RecordBatch(arrowSchema, columns: columns))
     }
 
-    public func fromStream( // swiftlint:disable:this function_body_length
+    public func fromMemoryStream( // swiftlint:disable:this 
function_body_length

Review Comment:
   Could you add a comment that explains the difference between 
`fromMemoryStream` and `fromFileStream`?



##########
swift/Arrow/Sources/Arrow/ArrowReaderHelper.swift:
##########
@@ -289,3 +289,10 @@ func validateFileData(_ data: Data) -> Bool {
     let endString = String(decoding: data[(data.count - markerLength)...], as: 
UTF8.self)
     return startString == FILEMARKER && endString == FILEMARKER
 }
+
+func getUInt32(_ data: Data, offset: Int) -> UInt32 {
+    let token = data.withUnsafeBytes { rawBuffer in
+        rawBuffer.loadUnaligned(fromByteOffset: offset, as: UInt32.self)

Review Comment:
   Do we need a out-of-range offset check here?
   



##########
swift/Arrow/Sources/Arrow/ArrowReader.swift:
##########
@@ -216,7 +216,67 @@ public class ArrowReader { // swiftlint:disable:this 
type_body_length
         return .success(RecordBatch(arrowSchema, columns: columns))
     }
 
-    public func fromStream( // swiftlint:disable:this function_body_length
+    public func fromMemoryStream( // swiftlint:disable:this 
function_body_length
+        _ fileData: Data,
+        useUnalignedBuffers: Bool = false
+    ) -> Result<ArrowReaderResult, ArrowError> {
+        let result = ArrowReaderResult()
+        var offset: Int = 0
+        var length = getUInt32(fileData, offset: offset)
+        var streamData = fileData
+        var schemaMessage: org_apache_arrow_flatbuf_Schema?
+        while length != 0 {
+            if length == CONTINUATIONMARKER {
+                offset += Int(MemoryLayout<Int32>.size)

Review Comment:
   Is this the size of length data?
   How about using `UInt32` not `Int32` because length data is UInt32 not Int32?



-- 
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]

Reply via email to