nastra commented on code in PR #13386: URL: https://github.com/apache/iceberg/pull/13386#discussion_r2167090490
########## core/src/main/java/org/apache/iceberg/rest/ParserContext.java: ########## @@ -44,7 +44,7 @@ static Builder builder() { } static class Builder { - private Map<String, Object> data; + private final Map<String, Object> data; Review Comment: the `final` applies to the actual map instance and doesn't imply that the map is immutable. Looking at the code again I don't think it actually works currently when the builder's `add()` method is called because `data` is an empty immutable map. @singhpk234 shouldn't the `data` map rather be just an empty mutable map? ``` - private final Map<String, Object> data; - - private Builder() { - this.data = Collections.emptyMap(); - } + private final Map<String, Object> data = Maps.newHashMap(); ``` -- 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