willtemperley commented on code in PR #39:
URL: https://github.com/apache/arrow-swift/pull/39#discussion_r2476443452
##########
Arrow/Sources/Arrow/ArrowType.swift:
##########
@@ -165,14 +165,27 @@ public class ArrowTypeTimestamp: ArrowType {
}
}
-public class ArrowNestedType: ArrowType {
+public class ArrowTypeStruct: ArrowType {
let fields: [ArrowField]
public init(_ info: ArrowType.Info, fields: [ArrowField]) {
self.fields = fields
super.init(info)
}
}
+public class ArrowTypeList: ArrowType {
+ public let elementField: ArrowField
+
+ public init(_ elementField: ArrowField) {
+ self.elementField = elementField
+ super.init(ArrowType.ArrowList)
+ }
+
+ public convenience init(_ elementType: ArrowType, nullable: Bool = true) {
Review Comment:
it would probably be better to consistently use `isNullable` rather than
nullable.
##########
Arrow/Sources/Arrow/ArrowArrayBuilder.swift:
##########
@@ -169,11 +169,36 @@ public class StructArrayBuilder:
ArrowArrayBuilder<StructBufferBuilder, StructAr
let arrowData = try ArrowData(self.type, buffers: buffers,
children: childData, nullCount:
self.nullCount,
length: self.length)
- let structArray = try StructArray(arrowData)
+ let structArray = try NestedArray(arrowData)
return structArray
}
}
+public class ListArrayBuilder: ArrowArrayBuilder<ListBufferBuilder,
NestedArray> {
+ let valueBuilder: any ArrowArrayHolderBuilder
+
+ public override init(_ elementType: ArrowType) throws {
Review Comment:
This doesn't allow element nullability to be specified. I think this isn't
actually enforced (which is a separate issue) but the public API should reflect
the spec at least. I'd do something like this, directly providing the list type
which holds the `ArrowField` and therefore the nullability information;
```
public override init(_ arrowType: ArrowType) throws {
guard let arrowType = arrowType as? ArrowTypeList else {
throw ArrowError.invalid("Expected ArrowTypeList")
}
let arrowField = arrowType.elementField
self.valueBuilder = try ArrowArrayBuilders.loadBuilder(arrowType:
arrowField.type)
try super.init(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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
