rdblue commented on a change in pull request #2952:
URL: https://github.com/apache/iceberg/pull/2952#discussion_r698058669
##########
File path: api/src/main/java/org/apache/iceberg/types/PruneColumns.java
##########
@@ -77,43 +88,48 @@ public Type struct(Types.StructType struct, List<Type>
fieldResults) {
@Override
public Type field(Types.NestedField field, Type fieldResult) {
if (selected.contains(field.fieldId())) {
- return field.type();
+ if (field.type().isStructType() && !selectFullTypes) {
+ return projectSelectedStruct(fieldResult);
+ } else {
+ Preconditions.checkArgument(selectFullTypes ||
!field.type().isNestedType(),
+ "Cannot select partial List or Map types explicitly, %s:%s of type
%s was selected",
+ field.fieldId(), field.name(), field.type());
+ // Selected non-struct field
+ return field.type();
+ }
} else if (fieldResult != null) {
- // this isn't necessarily the same as field.type() because a struct may
not have all
- // fields selected.
- return fieldResult;
+ // This field wasn't selected but a subfield was so include that
+ return fieldResult;
}
return null;
}
@Override
public Type list(Types.ListType list, Type elementResult) {
if (selected.contains(list.elementId())) {
- return list;
- } else if (elementResult != null) {
- if (list.elementType() == elementResult) {
- return list;
- } else if (list.isElementOptional()) {
- return Types.ListType.ofOptional(list.elementId(), elementResult);
+ if (list.elementType().isStructType() && !selectFullTypes) {
Review comment:
I think this should be slightly different:
```java
if (selectFullTypes) {
return list;
} else if (list.elementType().isStructType()) {
StructType projectedStruct = projectSelectedStruct(elementResult);
return projectList(list, projectedStruct);
} else {
Precondtions.checkArgument(list.elementType().isPrimitive(), "Cannot
directly project list or map");
return list;
}
```
I think that makes it more understandable because `selectFullTypes` is
handled first and so we know that the remaining cases are not selecting full
types. It also handles lists of lists because of the argument check. I think
that this was allowing an inner list to be selected directly.
--
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]
