ctubbsii commented on PR #2854: URL: https://github.com/apache/accumulo/pull/2854#issuecomment-1212299115
Okay, it looks like there are several issues, and this PR isn't going to cut it: 1. compareTo and equals and hashCode currently rely on the implementations in the base class. This makes it unreliable for using the StoredTabletFile class in data structures in the context of requiring separate entries for each metadata entry when it is found stored non-normalized. 2. Overriding any of these methods can break existing code that relies on the current behavior. But, the current behavior might be broken, too (because of bullet point number 1 above). 3. There are a few cases where we place TabletFile in the same data structure as the StoredTabletFile, requiring them to have cohesive implementations for these methods. 4. ScanServerRefTabletFile complicates things further, because it overrides equals and hashCode, forcing incompatibility with the base class implementation, if these happen to get stored in the same data structure. I had considered removing the compareTo method, and defining an explicit comparator. That would address the places where we are trying to compare using the natural ordering of the base class and the subclass. But, it doesn't address these other issues. Creating an interface at the base might help make things less confusing... I'm not sure. Either way, it looks like we're going to have to dig through every use of these classes in any kind of data structure, to evaluate for correct reliance on the equals, hashCode, and compareTo implementations. Closing this PR, because it's not the right fix. -- 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]
