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.
In fact, this specific exception will never fire outside of tests. The above
call to getAndCallByName will "fail" (return null) for custom filters, and
cleanly fallback to the below older 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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
