[
https://issues.apache.org/jira/browse/CALCITE-4096?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17152482#comment-17152482
]
Haisheng Yuan commented on CALCITE-4096:
----------------------------------------
I think I was not making myself clear. By "Outside of Calcite, it sounds insane
to use Calcite's Pair as third-party's customized Map class's map entry.", I
meant a customized Map class that use Calcite's Pair as map entry like the
following:
{code:java}
public class MyMap<K, V> extends AbstractMap {
private Set<Pair<K, V>> entrySet = new EntrySetUsingCalcitePair<>();
.......
}
{code}
If there are 2 maps with exact same keys and values, 1 is instance of MyMap,
the other is Java standard HashMap. I am expecting the 2 maps having the same
hashCode and equals to each other. Now the hashCode is indeed the same, but
HashMap.equals(MyMap) returns true, but not necessarily vice versa, because
MyMap equals may just check the entrySet are equals or not, like how Guava does:
{code:java}
/**
* An implementation of {@link Map#equals}.
*/
static boolean equalsImpl(Map<?, ?> map, Object object) {
if (map == object) {
return true;
} else if (object instanceof Map) {
Map<?, ?> o = (Map<?, ?>) object;
return map.entrySet().equals(o.entrySet());
}
return false;
}
{code}
MyMap's entrySet won't equal with HashMap's entrySet, because Pair.equals
breaks Map.Entry#equals requirement. We will have HashMap.equals(MyMap) returns
true, but MyMap.equals(HashMap) returns false.
Apache commons Pair, on the other hand, fully follow Map.Entry protocol, both
hashCode and equals. Similarly when the 2 instances of commons Pair and Calcite
Pair have the same key and value, we have
commonsPair.equals(calcitePair) returns true, but
calcitePair.equals(commonsPair) returns false.
In that sense, it sounds insane to use calcite Pair as a general Map.Entry
inside Map container outside of Calcite. Of course, by implementing Map.Entry,
the Calcite Pair can enjoy the benefits of general collection framework
utility, like {{ImmutableMap.copyOf}}, note that it just copies the entry's key
value, doesn't care about whether the entries passed in has same hashCode or
not.
{{RelDataTypeFactory.createStructType}} also demonstrated that it doesn't care
the entry follows Map.Entry's hashCode and equals protocol or not, because
RelDataTypeFieldImpl already breaks it.
So I don't think it makes much sense to strictly follow hashCode algorithm of
Map.Entry without doing the same for equals method, unless we change Calcite's
Pair, to be able to equal with any implementation class of Map.Entry, instead
of limiting to Calcite's own Pair class.
> Change Pair.hashCode() not to use XOR
> -------------------------------------
>
> Key: CALCITE-4096
> URL: https://issues.apache.org/jira/browse/CALCITE-4096
> Project: Calcite
> Issue Type: Improvement
> Components: core
> Reporter: Haisheng Yuan
> Priority: Major
>
> JDK Map Entry uses XOR maybe because it is unlikely to use Map Entry as the
> HashMap key.
> But Pair in Calcite is a general data structure, it is used in several places
> as the key of HashMap/HashSet. XOR is not a good candidate for hash
> algorithm, it is more likely to generate hash collision than simple prime
> multiplication, especially when pair.left equals pair.right.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
