abhishekagarwal87 commented on pull request #11908:
URL: https://github.com/apache/druid/pull/11908#issuecomment-979080833


   
   
   
   > Minor comment: Currently, deserializing the list of queries in JSON is 
done by converting it into instance of ArrayNode, since a simple 
List<Map<String, Object>> was losing the information about `queryType` 
[referenced 
code](https://github.com/apache/druid/pull/11908/files#diff-48bb0966fc1146b9b29e18a06ad3f0c38ba62fd2603905d33ecda842d06d98ddR453),
 
[stackoverflow](https://stackoverflow.com/questions/34193177/why-does-jackson-polymorphic-serialization-not-work-in-lists)
 I have recently found a way to not use these by creating an inner class
   > 
   > ```
   > private static class ExplainOutputNode
   >   {
   >     @JsonProperty
   >     Query query;
   > 
   >     @JsonProperty
   >     RowSignature signature;
   > 
   >     public ExplainOutputNode(RowSignature signature, Query query) {
   >       this.signature = signature;
   >       this.query = query;
   >     }
   >   }
   > 
   >     String outputString =  jsonMapper.writerFor(new 
TypeReference<List<ExplainOutputNode>>()
   >     {
   >     }).writeValueAsString(queryList);
   > ```
   > 
   > which frees the referenced code from usage of `ArrayNode` and 
`ObjectNode`. Should that be a preferred approach to the current one?
   
   I think that the current approach is fine since the new approach means 
creating a new type. If in future, we do similar deserialization in other 
places, we can use a custom type specifically for explain output. 


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