timothyw553 commented on code in PR #16913:
URL: https://github.com/apache/iceberg/pull/16913#discussion_r3633130528
##########
spark/v4.0/spark/src/main/java/org/apache/iceberg/spark/data/vectorized/ColumnVectorWithFilter.java:
##########
@@ -137,14 +142,21 @@ public ColumnVector getChild(int ordinal) {
if (children == null) {
synchronized (this) {
if (children == null) {
+ int numChildren;
if (dataType() instanceof StructType) {
StructType structType = (StructType) dataType();
- this.children = new ColumnVectorWithFilter[structType.length()];
- for (int index = 0; index < structType.length(); index++) {
- children[index] = new
ColumnVectorWithFilter(delegate.getChild(index), rowIdMapping);
- }
+ numChildren = structType.length();
+ } else if (dataType() instanceof CalendarIntervalType) {
+ numChildren = NUM_INTERVAL_CHILDREN;
+ } else if (dataType() instanceof VariantType) {
+ numChildren = NUM_VARIANT_CHILDREN;
} else {
- throw new UnsupportedOperationException("Unsupported nested type:
" + dataType());
+ return delegate.getChild(ordinal);
Review Comment:
good point. Updated in a7bb903 to handle `ArrayType` and `MapType`
explicitly while restoring the fail-fast exception for unknown child types. I
applied this consistently across Spark 3.5, 4.0, and 4.1 and added regression
coverage.
##########
spark/v4.0/spark/src/main/java/org/apache/iceberg/spark/data/vectorized/ColumnVectorWithFilter.java:
##########
@@ -137,14 +142,21 @@ public ColumnVector getChild(int ordinal) {
if (children == null) {
synchronized (this) {
if (children == null) {
+ int numChildren;
if (dataType() instanceof StructType) {
StructType structType = (StructType) dataType();
- this.children = new ColumnVectorWithFilter[structType.length()];
- for (int index = 0; index < structType.length(); index++) {
- children[index] = new
ColumnVectorWithFilter(delegate.getChild(index), rowIdMapping);
- }
+ numChildren = structType.length();
+ } else if (dataType() instanceof CalendarIntervalType) {
+ numChildren = NUM_INTERVAL_CHILDREN;
+ } else if (dataType() instanceof VariantType) {
+ numChildren = NUM_VARIANT_CHILDREN;
Review Comment:
thanks for flagging this. I rebased onto the latest `main`, including
#16292, and preserved its Variant child-vector remapping in Spark 4.0 and 4.1.
--
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]
