You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by GitBox <gi...@apache.org> on 2022/09/09 17:46:15 UTC

[GitHub] [druid] gianm opened a new pull request, #13064: Cleaner JSON for various input sources and formats.

gianm opened a new pull request, #13064:
URL: https://github.com/apache/druid/pull/13064

   Add JsonInclude to various properties, to avoid population of default values in serialized JSON.
   
   Also fixes a bug in OrcInputFormat: it was not writing binaryAsString, so the property would be lost on serde.


-- 
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


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

Posted by GitBox <gi...@apache.org>.
FrankChen021 commented on code in PR #13064:
URL: https://github.com/apache/druid/pull/13064#discussion_r967575315


##########
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:
   I'm wondering if we can configure the `DefaultObjectMapper` to exclude all null value in global so that we don't need to add such annotation for every get method?
   
   ```java
   mapper.setSerializationInclusion(JsonInclude.Include.NON_NULL);
   ```



-- 
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


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

Posted by 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


[GitHub] [druid] gianm merged pull request #13064: Cleaner JSON for various input sources and formats.

Posted by GitBox <gi...@apache.org>.
gianm merged PR #13064:
URL: https://github.com/apache/druid/pull/13064


-- 
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