kbendick commented on code in PR #4641:
URL: https://github.com/apache/iceberg/pull/4641#discussion_r859334162
##########
core/src/main/java/org/apache/iceberg/TableMetadataParser.java:
##########
@@ -301,6 +301,10 @@ static TableMetadata fromJson(FileIO io, InputFile file,
JsonNode node) {
return fromJson(io, file.location(), node);
}
+ public static TableMetadata fromJson(JsonNode node) {
+ return fromJson(null, (String) null, node);
+ }
Review Comment:
This parser will return a TableMetadata that presently can raise an NPE if
the changes of a particular snapshot are queried, as the FileIO object is not
present.
Unfortunately, the needed `FileIO` is not available in the scope of the
serializers used by Jackson with the `RESTCatalog`.
To get around this, in a follow up PR we will take the functions in
`BaseSnapshot` that _do_ potentially depend on `FileIO` and have them take an
explicit `FileIO` object as an argument.
This will allow for catalogs to pick the correct `FileIO` when needed, and
removes the need for this class to carry around as much state (which it passes
to children in its snapshot log etc). I believe this is a better architecture
overall, as the correct `FileIO` (that's properly configured etc) is even more
lazily evaluated.
If the `TableMetadata` doesn't have a call to access added files, deleted
files, or added / deleted manifests, or if there is no Snapshot log, then the
object is correct and useable just fine.
As there's no code that is actively using this path at present (or the code
that calls to it), it should be fine to merge in stages.
--
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]
