kfaraz commented on code in PR #18547:
URL: https://github.com/apache/druid/pull/18547#discussion_r2374085705


##########
sql/src/main/java/org/apache/druid/sql/calcite/schema/SystemSchema.java:
##########
@@ -180,6 +180,7 @@ public class SystemSchema extends AbstractSchema
       .add("max_size", ColumnType.LONG)
       .add("is_leader", ColumnType.LONG)
       .add("start_time", ColumnType.STRING)
+      .add("labels", ColumnType.NESTED_DATA)

Review Comment:
   Oh, I see what's happening here. Sorry, I had missed this in the initial 
pass.
   You _will_ need to use a `jsonMapper` and keep the column type as `STRING`
   since that is what `SegmentsTable` is doing as well but lazilly.
   
   The segments table uses a list of `SEGMENTS_JSON_FIELDS` to identify which 
fields to serialize as json.
   Ideally, you would want to do something similar to avoid invoking 
`jsonMapper.writeValueAsString()` every time
   we receive a query for `sys.servers` so that we serialize that field only 
when needed.
   
   But for now, you may just stick to what you had originally that is:
   `node.getLabels() == null ? null : 
jsonMapper.writeValueAsString(node.getLabels())`
   
   The perf improvement can be done later.
   
   Sorry for the confusion 😅 !



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