kbendick commented on code in PR #5275:
URL: https://github.com/apache/iceberg/pull/5275#discussion_r922418866
##########
core/src/main/java/org/apache/iceberg/rest/responses/LoadTableResponse.java:
##########
@@ -37,6 +42,56 @@
*/
public class LoadTableResponse implements RESTResponse {
+ private static final String METADATA_LOCATION = "metadata-location";
+ private static final String METADATA = "metadata";
+ private static final String CONFIG = "config";
+
+ public static void toJson(LoadTableResponse loadTable, JsonGenerator
generator) throws IOException {
+ generator.writeStartObject();
+
+ generator.writeStringField(METADATA_LOCATION, loadTable.metadataLocation);
+
+ generator.writeFieldName(METADATA);
+ TableMetadataParser.toJson(loadTable.metadata, generator);
+
+ generator.writeObjectFieldStart(CONFIG);
+ for (Map.Entry<String, String> keyValue : loadTable.config.entrySet()) {
+ generator.writeStringField(keyValue.getKey(), keyValue.getValue());
Review Comment:
Technically speaking, when using Jackson's `JsonGenerator`, if the map
_does_ allow for `null` values, I think something like the following is needed:
```java
if (keyValue.getValue() != null) {
generator.writeStringField(keyValue.getKey(), keyValue.getValue());
} else {
generator.writeNullField(keyValue.getKey());
}
```
Usually we tend to convert input Maps to Guava's `ImmutableMap` using
`ImmutableMap.copyOf()` like here in
[TableMetadata](https://github.com/apache/iceberg/blob/64ed2a7e2b5f7acce921e6d2c40d97ef276c44e8/core/src/main/java/org/apache/iceberg/TableMetadata.java#L1244-L1245).
Guava's `ImmutableMap` does not allow for `null` values (and its `copyOf`
method is a no-op if the map in question is already an `ImmutableMap` so it's
ok to use regularly).
But `LoadTableResponse` actually does not [return an immutable map
copy](https://github.com/apache/iceberg/blob/95f9adba722d192f79f26fdf94d90fc8d8e4bc50/core/src/main/java/org/apache/iceberg/rest/responses/LoadTableResponse.java#L67-L69),
in case the person defining the REST catalogs backend has assigned some
special meaning to `null` values in their configuration object.
--
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]
