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

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

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



##########
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:
       I think I agree with the decision to not change the behavior of this method, even though the opposite of "select" behavior would be to fully remove a struct when its ID is passed in `fieldIds`.
   
   But I don't think that `project` is quite correct either. Consider the example schema `1: id bigint, 2: location struct<3: lat double, 4: long double>`. Previously, `selectNot(t, set(3, 4))` would produce `1: id bigint` and omit the location entirely. Using project with the updated `GetProjectedIds`, the projected ID set will be {1, 2, 3, 4} and not `{1, 3, 4}`. That would result in the same call producing `1: id bigint, 2: location struct<>`, which introduces a new bug because now there is an unexpected extra field.
   
   To clean this up, I think we need a version of `GetProjectedIds` that doesn't select structs and uses the old behavior.




-- 
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