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: [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]

Reply via email to