[GitHub] [druid] gianm commented on a diff in pull request #13064: Cleaner JSON for various input sources and formats.

[GitBox <gi...@apache.org>]

gianm commented on code in PR #13064:
URL: https://github.com/apache/druid/pull/13064#discussion_r967858135


##########
core/src/main/java/org/apache/druid/data/input/impl/CloudObjectInputSource.java:
##########
@@ -83,26 +84,30 @@ public CloudObjectInputSource(
   }
 
   @JsonProperty
+  @JsonInclude(JsonInclude.Include.NON_NULL)

Review Comment:
   Interesting idea. It would save a lot of thinking about specific properties.
   
   My first thought is that I wonder if it is always safe? Certainly, `NON_DEFAULT` and `NON_EMPTY` across the board aren't always safe; there are various classes where passing in `null` vs `false` for a Boolean, or `null` vs `0` for an Integer, mean different things. `NON_NULL` seems safe at first glance, although I haven't thought about it very hard yet.
   
   My second thought is that I wonder if it can be overridden in some cases. There are some cases where serializing a null value is more beautiful, like a selector filter that has `value` set to `null`. (Otherwise, it would serialize as `{"type":"selector"}` with no value, which while correct, definitely looks strange.)



-- 
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: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org