You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "abandy (via GitHub)" <gi...@apache.org> on 2023/05/26 02:53:51 UTC

[GitHub] [arrow] abandy opened a new pull request, #35774: MINOR: [Swift] bug fixes and change reader/writer to user Result type

abandy opened a new pull request, #35774:
URL: https://github.com/apache/arrow/pull/35774

   Changes:
   - Changed Reader and Writer to use Result<T, Error> instead of throwing.  Since the Error is an enum the handling of the error is enforced.
   - Update RecordBatch to accept only a single array per column instead of chunked arrays (which could cause problems when writing out)
   - Added ability for Table to be created from RecordBatches
   - Fixed a bug with indexing in Chunked array with many arrays 
   - Added tests for the above changes.
   


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


[GitHub] [arrow] ursabot commented on pull request #35774: GH-35788: [Swift] bug fixes and change reader/writer to user Result type

Posted by "ursabot (via GitHub)" <gi...@apache.org>.
ursabot commented on PR #35774:
URL: https://github.com/apache/arrow/pull/35774#issuecomment-1569326463

   Benchmark runs are scheduled for baseline = 130f9e981aa98c25de5f5bfe55185db270cec313 and contender = d14b42ac6b25823042b9da83ee16615579a81fa1. d14b42ac6b25823042b9da83ee16615579a81fa1 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
   Conbench compare runs links:
   [Failed] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/67c9d1e16d0b4ec689babed3f28c2376...51122813cdce4e9d8476b4217a670e33/)
   [Failed] [test-mac-arm](https://conbench.ursa.dev/compare/runs/fdc7a4395794469897c482cabbe18e6b...6b9bb1a551d84a62a27ec5f453bf5aef/)
   [Failed] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/82153ae9d0aa4c6a8a640e499674918d...d7f6d9e75931401c91a059b09d4a51f4/)
   [Failed] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/c314678f022b411ea2098ed696e8c5d5...97f887779c4e4f42835282d76931e3ff/)
   Buildkite builds:
   [Finished] [`d14b42ac` ec2-t3-xlarge-us-east-2](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/2943)
   [Failed] [`d14b42ac` test-mac-arm](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/2979)
   [Finished] [`d14b42ac` ursa-i9-9960x](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/2944)
   [Failed] [`d14b42ac` ursa-thinkcentre-m75q](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/2969)
   [Finished] [`130f9e98` ec2-t3-xlarge-us-east-2](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/2942)
   [Failed] [`130f9e98` test-mac-arm](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/2978)
   [Finished] [`130f9e98` ursa-i9-9960x](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/2943)
   [Failed] [`130f9e98` ursa-thinkcentre-m75q](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/2968)
   Supported benchmarks:
   ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
   test-mac-arm: Supported benchmark langs: C++, Python, R
   ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
   ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
   


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


[GitHub] [arrow] kou commented on a diff in pull request #35774: GH-35788: [Swift] bug fixes and change reader/writer to user Result type

Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on code in PR #35774:
URL: https://github.com/apache/arrow/pull/35774#discussion_r1206767131


##########
swift/Arrow/Sources/Arrow/ArrowArray.swift:
##########
@@ -17,7 +17,60 @@
 
 import Foundation
 
-public class ArrowArray<T> {
+public class ArrowArrayHolder {
+    public let type: ArrowType.Info
+    public let length: UInt
+    public let nullCount: UInt
+    public let holder: Any
+    public let getBufferData: () -> [Data]
+    public let getBufferDataSizes: () -> [Int]
+    private let getArrowColumn: (ArrowField, [ArrowArrayHolder]) throws -> ArrowColumn
+    public init<T>(_ arrowArray: ArrowArray<T>) {
+        self.holder = arrowArray
+        self.length = arrowArray.length
+        self.type = arrowArray.arrowData.type
+        self.nullCount = arrowArray.nullCount
+        self.getBufferData = {() -> [Data] in
+            var bufferData = [Data]()
+            for buffer in arrowArray.arrowData.buffers {
+                bufferData.append(Data())
+                buffer.append(to: &bufferData[bufferData.count - 1])
+            }
+
+            return bufferData;
+        }
+        
+        self.getBufferDataSizes = {() -> [Int] in
+            var bufferDataSizes = [Int]()
+            for buffer in arrowArray.arrowData.buffers {
+                bufferDataSizes.append(Int(buffer.capacity))
+            }
+
+            return bufferDataSizes
+        }
+        
+        self.getArrowColumn = {(field: ArrowField, arrayHolders: [ArrowArrayHolder]) throws -> ArrowColumn in
+            var arrays = [ArrowArray<T>]()
+            for arrayHolder in arrayHolders {
+                if let array = arrayHolder.holder as? ArrowArray<T> {
+                    arrays.append(array)
+                }
+            }
+            
+            return ArrowColumn(field, chunked: ChunkedArrayHolder(try ChunkedArray<T>(arrays)))
+        }
+    }
+    
+    public static func makeArrowColumn(_ field: ArrowField, holders: [ArrowArrayHolder]) -> Result<ArrowColumn, ArrowError> {
+        do {
+            return .success(try holders[0].getArrowColumn(field, holders))
+        }catch {

Review Comment:
   ```suggestion
           } catch {
   ```



##########
swift/Arrow/Sources/Arrow/ArrowArray.swift:
##########
@@ -17,7 +17,60 @@
 
 import Foundation
 
-public class ArrowArray<T> {
+public class ArrowArrayHolder {
+    public let type: ArrowType.Info
+    public let length: UInt
+    public let nullCount: UInt
+    public let holder: Any

Review Comment:
   How about renaming this to `array`?
   
   ```suggestion
       public let array: Any
   ```
   
   I feel that `ArrowArrayHolder.holder` is strange.



##########
swift/Arrow/Sources/Arrow/ArrowReader.swift:
##########
@@ -138,26 +158,36 @@ public class ArrowReader {
             let message = org_apache_arrow_flatbuf_Message.getRootAsMessage(bb: mbb)
             switch message.headerType {
             case .recordbatch:
-                let recordBatch = try loadRecordBatch(message, schema: footer.schema!,
-                                                      data: fileData, messageEndOffset: messageEndOffset)
-                recordBatchs.append(recordBatch)
+                do {
+                    let recordBatch = try loadRecordBatch(message, schema: footer.schema!,
+                                                     data: fileData, messageEndOffset: messageEndOffset).get()

Review Comment:
   ```suggestion
                                                             data: fileData, messageEndOffset: messageEndOffset).get()
   ```



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


[GitHub] [arrow] abandy commented on pull request #35774: MINOR: [Swift] bug fixes and change reader/writer to user Result type

Posted by "abandy (via GitHub)" <gi...@apache.org>.
abandy commented on PR #35774:
URL: https://github.com/apache/arrow/pull/35774#issuecomment-1564243285

   > Could you open a new issue for this because this is not a MINOR change?
   > 
   > See also: https://github.com/apache/arrow/blob/main/CONTRIBUTING.md#minor-fixes
   
   I see :).  I will open an issue, thank you!


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


[GitHub] [arrow] kou merged pull request #35774: GH-35788: [Swift] bug fixes and change reader/writer to user Result type

Posted by "kou (via GitHub)" <gi...@apache.org>.
kou merged PR #35774:
URL: https://github.com/apache/arrow/pull/35774


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


[GitHub] [arrow] ursabot commented on pull request #35774: GH-35788: [Swift] bug fixes and change reader/writer to user Result type

Posted by "ursabot (via GitHub)" <gi...@apache.org>.
ursabot commented on PR #35774:
URL: https://github.com/apache/arrow/pull/35774#issuecomment-1570049225

   ['Python', 'R'] benchmarks have high level of regressions.
   [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/82153ae9d0aa4c6a8a640e499674918d...d7f6d9e75931401c91a059b09d4a51f4/)
   


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


[GitHub] [arrow] github-actions[bot] commented on pull request #35774: GH-35788: [Swift] bug fixes and change reader/writer to user Result type

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #35774:
URL: https://github.com/apache/arrow/pull/35774#issuecomment-1564250259

   :warning: GitHub issue #35788 **has been automatically assigned in GitHub** to PR creator.


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


[GitHub] [arrow] github-actions[bot] commented on pull request #35774: GH-35788: [Swift] bug fixes and change reader/writer to user Result type

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #35774:
URL: https://github.com/apache/arrow/pull/35774#issuecomment-1564250228

   * Closes: #35788


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


[GitHub] [arrow] kou commented on pull request #35774: MINOR: [Swift] bug fixes and change reader/writer to user Result type

Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on PR #35774:
URL: https://github.com/apache/arrow/pull/35774#issuecomment-1563750875

   Could you open a new issue for this because this is not a MINOR change?
   
   See also: https://github.com/apache/arrow/blob/main/CONTRIBUTING.md#minor-fixes


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


[GitHub] [arrow] abandy commented on a diff in pull request #35774: GH-35788: [Swift] bug fixes and change reader/writer to user Result type

Posted by "abandy (via GitHub)" <gi...@apache.org>.
abandy commented on code in PR #35774:
URL: https://github.com/apache/arrow/pull/35774#discussion_r1206882051


##########
swift/Arrow/Sources/Arrow/ArrowArray.swift:
##########
@@ -17,7 +17,60 @@
 
 import Foundation
 
-public class ArrowArray<T> {
+public class ArrowArrayHolder {
+    public let type: ArrowType.Info
+    public let length: UInt
+    public let nullCount: UInt
+    public let holder: Any

Review Comment:
   Gotcha, yeah, good catch, I will update.



##########
swift/Arrow/Sources/Arrow/ArrowArray.swift:
##########
@@ -17,7 +17,60 @@
 
 import Foundation
 
-public class ArrowArray<T> {
+public class ArrowArrayHolder {
+    public let type: ArrowType.Info
+    public let length: UInt
+    public let nullCount: UInt
+    public let holder: Any
+    public let getBufferData: () -> [Data]
+    public let getBufferDataSizes: () -> [Int]
+    private let getArrowColumn: (ArrowField, [ArrowArrayHolder]) throws -> ArrowColumn
+    public init<T>(_ arrowArray: ArrowArray<T>) {
+        self.holder = arrowArray
+        self.length = arrowArray.length
+        self.type = arrowArray.arrowData.type
+        self.nullCount = arrowArray.nullCount
+        self.getBufferData = {() -> [Data] in
+            var bufferData = [Data]()
+            for buffer in arrowArray.arrowData.buffers {
+                bufferData.append(Data())
+                buffer.append(to: &bufferData[bufferData.count - 1])
+            }
+
+            return bufferData;
+        }
+        
+        self.getBufferDataSizes = {() -> [Int] in
+            var bufferDataSizes = [Int]()
+            for buffer in arrowArray.arrowData.buffers {
+                bufferDataSizes.append(Int(buffer.capacity))
+            }
+
+            return bufferDataSizes
+        }
+        
+        self.getArrowColumn = {(field: ArrowField, arrayHolders: [ArrowArrayHolder]) throws -> ArrowColumn in
+            var arrays = [ArrowArray<T>]()
+            for arrayHolder in arrayHolders {
+                if let array = arrayHolder.holder as? ArrowArray<T> {
+                    arrays.append(array)
+                }
+            }
+            
+            return ArrowColumn(field, chunked: ChunkedArrayHolder(try ChunkedArray<T>(arrays)))
+        }
+    }
+    
+    public static func makeArrowColumn(_ field: ArrowField, holders: [ArrowArrayHolder]) -> Result<ArrowColumn, ArrowError> {
+        do {
+            return .success(try holders[0].getArrowColumn(field, holders))
+        }catch {

Review Comment:
   Will update.



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


[GitHub] [arrow] abandy commented on a diff in pull request #35774: GH-35788: [Swift] bug fixes and change reader/writer to user Result type

Posted by "abandy (via GitHub)" <gi...@apache.org>.
abandy commented on code in PR #35774:
URL: https://github.com/apache/arrow/pull/35774#discussion_r1206882486


##########
swift/Arrow/Sources/Arrow/ArrowReader.swift:
##########
@@ -138,26 +158,36 @@ public class ArrowReader {
             let message = org_apache_arrow_flatbuf_Message.getRootAsMessage(bb: mbb)
             switch message.headerType {
             case .recordbatch:
-                let recordBatch = try loadRecordBatch(message, schema: footer.schema!,
-                                                      data: fileData, messageEndOffset: messageEndOffset)
-                recordBatchs.append(recordBatch)
+                do {
+                    let recordBatch = try loadRecordBatch(message, schema: footer.schema!,
+                                                     data: fileData, messageEndOffset: messageEndOffset).get()

Review Comment:
   Will update.



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