keith-turner commented on a change in pull request #313: ACCUMULO-4726 Add
Value.contentEquals(byte[]) method
URL: https://github.com/apache/accumulo/pull/313#discussion_r145976273
##########
File path: core/src/main/java/org/apache/accumulo/core/data/Value.java
##########
@@ -242,16 +242,16 @@ public int compareTo(final byte[] that) {
@Override
public boolean equals(Object right_obj) {
- // compare with byte[] for backwards compatibility, but this is generally
a pretty bad practice
- if (right_obj instanceof byte[]) {
- return compareTo((byte[]) right_obj) == 0;
Review comment:
Any code that was relying on this will break in surprising ways that are
hard to debug or even detect. The worst case for this change is that it causes
a silent failure in user code that corrupts data that may not be detected for
long periods of time. The exception is a solution to this worst case.
Throwing an exception is not ideal, but neither was this original code.
The code has been like this for close to 10 years. For that time period, its
not unreasonable to think that a project may have relied on this behavior.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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
