RRXXZZYY commented on code in PR #5603:
URL: https://github.com/apache/datafusion-comet/pull/5603#discussion_r3901733172
##########
spark/src/main/scala/org/apache/comet/vector/NativeUtil.scala:
##########
@@ -322,6 +333,149 @@ class NativeUtil extends AutoCloseable {
}
object NativeUtil {
+
+ /**
+ * Create a vector whose physical struct children remain positional when the
exported Arrow
+ * schema contains duplicate names. Arrow's default struct factory indexes
children by name and
+ * collapses such fields.
+ */
+ private[comet] def createVector(field: Field, allocator: BufferAllocator):
FieldVector = {
+ val runtimeField = fieldForAllocation(field)
+ createPinnedVector(runtimeField, field, allocator)
+ }
+
+ /**
+ * Preserve Arrow's default allocation path unless a duplicate-name struct
needs positional
+ * runtime children. This is called for every imported column of every
native batch.
+ */
+ private[comet] def createVectorForImport(
+ field: Field,
+ allocator: BufferAllocator): FieldVector = {
+ val runtimeField = fieldForAllocation(field)
+ if (runtimeField eq field) {
+ field.createVector(allocator).asInstanceOf[FieldVector]
+ } else {
+ createPinnedVector(runtimeField, field, allocator)
+ }
+ }
+
+ private def createPinnedVector(
+ runtimeField: Field,
+ exportField: Field,
+ allocator: BufferAllocator): FieldVector = {
+ exportField.getType match {
+ case _: ArrowType.List | _: ArrowType.LargeList | _:
ArrowType.FixedSizeList =>
+ val vector = new RenamedListVector(runtimeField, exportField,
allocator)
+ vector.initializeChildrenFromFields(runtimeField.getChildren)
+ vector
+ case _: ArrowType.Map =>
+ val vector = new RenamedMapVector(runtimeField, exportField, allocator)
+ vector.initializeChildrenFromFields(runtimeField.getChildren)
+ vector
+ case _: ArrowType.Struct =>
+ val vector = new RenamedStructVector(runtimeField, exportField,
allocator)
+ // The Field-based StructVector constructor creates the direct
children. Initialize each
+ // child's descendants from the runtime schema without adding the
direct children twice.
+ val runtimeChildren = runtimeField.getChildren
+ var ordinal = 0
+ while (ordinal < runtimeChildren.size()) {
+ vector
+ .getChildByOrdinal(ordinal)
+ .asInstanceOf[FieldVector]
+
.initializeChildrenFromFields(runtimeChildren.get(ordinal).getChildren)
Review Comment:
Thanks, this prediction was correct. I added an ordinary-dispatcher
regression for `from_json(j, 'a INT, A INT')`; on the previous head it failed
with a null second child at `NativeUtil.createPinnedVector`.
In `0aa2e532`, `RenamedStructVector` now starts from the name/`FieldType`
constructor with no direct children, then
`initializeChildrenFromFields(runtimeField.getChildren)` materializes each
child in order before `getField` exposes the exported metadata. This avoids the
field-based constructor's lower-cased writer cache rather than trying to repair
its partially materialized children afterward.
Fresh current-head results: the new regression passes 1/1 on Spark 3.4 and
3.5; `NativeUtilSuite` passes 11/11 on both; and `create_named_struct` passes
1/1 on both. Full CI is still awaiting first-time-contributor workflow approval.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
