rdblue commented on code in PR #13039:
URL: https://github.com/apache/iceberg/pull/13039#discussion_r2241246122
##########
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) {
Review Comment:
Javadoc for the `visit` method says that the field results are traversed
when the `Iterable` passed here is consumed:
> Structs are passed an {@link Iterable} that traverses child fields during
iteration.
The intent of this structure is to consume these `Iterable` instances while
working with the object, but this creates a new `Iterable` using `concat` that
is lazy. The result of returning the field results unconsumed is an
unpredictable (or certainly hard to understand) order when visiting fields.
I think that this does the right thing because it will create a structure of
iterables that contain the primitive field IDs first and it doesn't really
matter _when_ the results are consumed, but I think this structure makes the
code harder to understand than it needs to be.
I think I'd prefer a solution that keeps a single top-level list and adds
only the requested number of field IDs to it. Most of the code would be the
same, with each structure checking whether it contains primitives and adding
them in order, before traversing the children if more IDs need to be added.
--
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]
