rohangarg commented on issue #12912:
URL: https://github.com/apache/druid/issues/12912#issuecomment-1220486382
@BartMiki :
1. Generally, for performance measurement we use JMH benchmarks in the
`druid-benchmarks` module. But for this case, we only need to check once that
there is no absurd increase in the time. So, the approach of local UT (no need
to commit it) you mention sounds good to me.
2. I agree with your point that the recommended way to serialize an event
should be a part of a new issue. For the current change though, I think it
could be ok to have `sql` and `sqlContext` for `KafkaEmitter` too since the
removal of those keys wasn't intended FWIU. All the rest of the emitters also
have those keys (because they directly serialize `Event`), so having them would
maintain parity. But I don't have a strong opinion on it - if you and
@FrankChen021 think that maintaining the current compat is better then I'm good
with it too.
If we choose to allow the new extra keys way, then we could also do the
following code approach which maybe simpler :
1. convert the event object to a map using `jsonMapper.convertValue(event,
new TypeReference<Map<String, Object>>() {}` - this map also contains the
`queryType`
2. Put the additional keys in the map
3. Call `jsonMapper.writeValueAsString(mergedMap)` to get the JSON string
--
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]
