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

Reply via email to