himanshug commented on a change in pull request #7985: fail complex type
'serde' registration when registered type does not match expected type
URL: https://github.com/apache/incubator-druid/pull/7985#discussion_r303077516
##########
File path:
processing/src/main/java/org/apache/druid/segment/serde/ComplexMetrics.java
##########
@@ -38,12 +38,18 @@ public static ComplexMetricSerde getSerdeForType(String
type)
return complexSerializers.get(type);
}
- public static void registerSerde(String type, Supplier<ComplexMetricSerde>
serdeSupplier)
+ public static <T extends ComplexMetricSerde> void registerSerde(String type,
Class<T> clazz, Supplier<T> serdeSupplier)
{
- if (ComplexMetrics.getSerdeForType(type) == null) {
- if (complexSerializers.containsKey(type)) {
- throw new ISE("Serializer for type[%s] already exists.", type);
+ if (complexSerializers.containsKey(type)) {
+ if
(!complexSerializers.get(type).getClass().getCanonicalName().equals(clazz.getCanonicalName()))
{
Review comment:
hmmm...thanks for that link, read through the answer there, I see that in
case of arrays `getCanonicalName()` returns something more recognizable however
it returning null for anonymous class is a turnoff for me as that leads to NPE
and causes bugs.
I don't think that output of `getName()` is totally unrecognizable for
toString(). So I still think it is better to make it a rule and use
`getName()` even in toString().
Also, I am not sure but I think those tools would allow occasional
exceptions to the rule e.g. if there is a critical situation where it is
absolute necessary to use `getCanonicalName()` then that could happen with some
comment for the tool to ignore that occurrence .
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
