danielcweeks commented on code in PR #12323:
URL: https://github.com/apache/iceberg/pull/12323#discussion_r1980212065
##########
parquet/src/main/java/org/apache/iceberg/parquet/Parquet.java:
##########
@@ -160,9 +161,10 @@ public static class WriteBuilder implements
InternalData.WriteBuilder {
private final Map<String, String> metadata = Maps.newLinkedHashMap();
private final Map<String, String> config = Maps.newLinkedHashMap();
private Schema schema = null;
+ private BiFunction<Integer, String, Type> variantShreddingFunc = null;
Review Comment:
I feel less and less comfortable about using functional references like this
in public interfaces. This really makes maintainability much harder because
we're not documenting what this represents. If we're going have a reference
like this, then we should just make an wrapper interface (e.g.
`VariantShreddingFunction`) that documents the parameters/return and allows for
better navigation of usage.
This is particularly prevalent in parquet and avro, but it even here we have
three different functional references that we're passing around. If it were
internal, I'd be fine, but these are public apis so we should do better.
--
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
