yyanyy commented on a change in pull request #2096:
URL: https://github.com/apache/iceberg/pull/2096#discussion_r560442043
##########
File path: core/src/main/java/org/apache/iceberg/TableMetadataParser.java
##########
@@ -242,6 +255,7 @@ public static TableMetadata read(FileIO io, InputFile file)
{
}
}
+ @SuppressWarnings("checkstyle:CyclomaticComplexity")
Review comment:
Yeah I imagine there would be some conflict when either of the PRs got
merged. I think the main problem with refactoring this is that a lot of the
things it parsed should be parsed together/validate each other; e.g. parse of
schema array has to come together with current schema id since both will either
present at the same time, or derived from a different field, and same goes for
sort order/partition spec. Since Java only allows returning a single object per
method, we either have to refactor the code a bit and ensure the logic flow
doesn't change and is readable, or let the new methods to return things like
`tuple<Integer, List<Schema>>` that I'm not sure which way is cleaner.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
