rdblue commented on code in PR #13039:
URL: https://github.com/apache/iceberg/pull/13039#discussion_r2241237393


##########
core/src/main/java/org/apache/iceberg/MetricsConfig.java:
##########
@@ -107,6 +115,81 @@ public static MetricsConfig forPositionDelete(Table table) 
{
     return new MetricsConfig(columnModes.build(), defaultMode);
   }
 
+  static Iterable<Integer> breadthFirstFieldPriority(Schema schema) {
+    return TypeUtil.visit(
+        schema,
+        new TypeUtil.CustomOrderSchemaVisitor<>() {
+
+          @Override
+          public Iterable<Integer> schema(Schema schema, 
Supplier<Iterable<Integer>> structResult) {
+            return structResult.get();
+          }
+
+          @Override
+          public Iterable<Integer> struct(
+              Types.StructType struct, Iterable<Iterable<Integer>> 
fieldResults) {
+
+            List<Integer> orderedFieldIds =
+                Lists.newArrayListWithExpectedSize(struct.fields().size());
+            for (Types.NestedField field : struct.fields()) {
+              if (field.type().isPrimitiveType()) {
+                orderedFieldIds.add(field.fieldId());
+              }
+            }
+            return Iterables.concat(orderedFieldIds, 
Iterables.concat(fieldResults));
+          }
+
+          @Override
+          public Iterable<Integer> field(
+              Types.NestedField field, Supplier<Iterable<Integer>> future) {
+            return future.get();
+          }
+
+          @Override
+          public Iterable<Integer> list(Types.ListType list, 
Supplier<Iterable<Integer>> future) {
+            List<Integer> returnValue = Lists.newArrayListWithCapacity(1);
+            if (list.elementType().isPrimitiveType()) {

Review Comment:
   I think that this can be simpler:
   
   ```java
     if (list.elementType.isPrimitiveType() || 
list.elementType().isVariantType()) {
       return ImmutableList.of(list.elementId());
     }
   
     return future.get();
   ```
   
   If the element is a primitive type (or a variant), then we don't need to get 
the value of the future because we already know that it is going to be 
`Collections.emptyList()`. I prefer that implementation because it cuts down on 
`Iterable` instances that concatenate things that we know are empty.



-- 
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-unsubscr...@iceberg.apache.org

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


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

Reply via email to