amogh-jahagirdar commented on code in PR #8723:
URL: https://github.com/apache/iceberg/pull/8723#discussion_r1347395043
##########
core/src/main/java/org/apache/iceberg/view/ViewMetadataParser.java:
##########
@@ -66,7 +67,9 @@ static void toJson(ViewMetadata metadata, JsonGenerator gen)
throws IOException
gen.writeStringField(VIEW_UUID, metadata.uuid());
gen.writeNumberField(FORMAT_VERSION, metadata.formatVersion());
gen.writeStringField(LOCATION, metadata.location());
- JsonUtil.writeStringMap(PROPERTIES, metadata.properties(), gen);
+ if (!metadata.properties().isEmpty()) {
Review Comment:
@ajantha-bhat Right, my point is I think what TableMetadata is doing already
is fine and we should be able to follow the same thing here.
In other words, why make it optional in the first place for views on the
write side when going through the Iceberg provided metadata writer?
I can see the argument for optional reads since there's no point failing
reading view metadata if it's missing properties, in case somebody messed with
metadata or has their own metadata writer that's OK to me because properties
are optional from a spec perspective. But I feel that on the write side the
Iceberg library provided metadata writer should just write out the properties.
--
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
