Re: [PR] GH-39720: [Swift] switch reader to use arrow field instead of proto for building arrays [arrow]

["kou (via GitHub)" <gi...@apache.org>]

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


##########
swift/Arrow/Sources/Arrow/ArrowType.swift:
##########
@@ -165,6 +167,48 @@ public class ArrowType {
             return ArrowType.ArrowUnknown
         }
     }
+
+    public static func getStride( // swiftlint:disable:this cyclomatic_complexity

Review Comment:
   How about making this a normal func (instance method) not a static func?



##########
swift/Arrow/Sources/Arrow/ArrowReaderHelper.swift:
##########
@@ -21,8 +21,9 @@ import Foundation
 private func makeBinaryHolder(_ buffers: [ArrowBuffer],
                               nullCount: UInt) -> Result<ArrowArrayHolder, ArrowError> {
     do {
-        let arrowData = try ArrowData(ArrowType(ArrowType.ArrowBinary), buffers: buffers,
-                                      nullCount: nullCount, stride: MemoryLayout<Int8>.stride)
+        let arrowType = ArrowType(ArrowType.ArrowBinary)
+        let arrowData = try ArrowData(arrowType, buffers: buffers,
+                                      nullCount: nullCount, stride: ArrowType.getStride(arrowType))

Review Comment:
   How about computing `stride:` in `ArrowData.init()` because `ArrowData.init()` can use the given `ArrowType`?



-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org