bbeaudreault commented on code in PR #5488: URL: https://github.com/apache/hbase/pull/5488#discussion_r1384834533
########## hbase-client/src/main/java/org/apache/hadoop/hbase/shaded/protobuf/ProtobufUtil.java: ########## @@ -1552,13 +1571,23 @@ public static ComparatorProtos.Comparator toComparator(ByteArrayComparable compa public static ByteArrayComparable toComparator(ComparatorProtos.Comparator proto) throws IOException { String type = proto.getName(); - String funcName = "parseFrom"; byte[] value = proto.getSerializedComparator().toByteArray(); + try { + ByteArrayComparable result = COMPARATORS.getAndCallByName(type, value); + if (result != null) { + return result; + } + + if (!ALLOW_FAST_REFLECTION_FALLTHROUGH) { + throw new IllegalStateException("Failed to deserialize comparator " + type + + " because fast reflection returned null and fallthrough is disabled"); + } Review Comment: I only added this so that I could write tests, since this is all static methods. I don't think we want a warn or counter here. How often it happens will depend on the usage of custom filters. If they don't use custom filters, this will never fail. If they use exclusively custom filters, then it will fail every time. It's not really a failure mode, more backwards compatibility handling. -- 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: issues-unsubscr...@hbase.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org