[GitHub] [iceberg] RussellSpitzer commented on a change in pull request #2953: API: Change GetProjectedIds to Return all Ids

[GitBox <gi...@apache.org>]

RussellSpitzer commented on a change in pull request #2953:
URL: https://github.com/apache/iceberg/pull/2953#discussion_r686352315



##########
File path: api/src/main/java/org/apache/iceberg/types/TypeUtil.java
##########
@@ -130,13 +130,13 @@ public static Schema select(Schema schema, Set<Integer> fieldIds) {
   public static Types.StructType selectNot(Types.StructType struct, Set<Integer> fieldIds) {
     Set<Integer> projectedIds = getIdsInternal(struct);
     projectedIds.removeAll(fieldIds);
-    return select(struct, projectedIds);
+    return project(struct, projectedIds);

Review comment:
       One issue here is selectNot doest not actually deselect children when a parent ID is not selected. Previously this is because getProjectedIDs (behind getIdsInternal) would not return parent struct ids, so removing it from the set of projectedIds would not do anything.
   
   Now It will not work because removing a parentID still leaves all child IDs. We could fix this but it would be a change in behavior from the previous code.




-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org