Github user d2r commented on a diff in the pull request:

    https://github.com/apache/storm/pull/641#discussion_r35120287
  
    --- Diff: 
storm-core/src/jvm/backtype/storm/grouping/PartialKeyGrouping.java ---
    @@ -65,7 +66,11 @@ public void prepare(WorkerTopologyContext context, 
GlobalStreamId stream, List<I
                     List<Object> selectedFields = outFields.select(fields, 
values);
                     ByteBuffer out = ByteBuffer.allocate(selectedFields.size() 
* 4);
                     for (Object o: selectedFields) {
    -                    out.putInt(o.hashCode());
    +                    if (o instanceof Object[]) {
    --- End diff --
    
    Yes, we need to catch the primitive array types as well somehow.
    
    Here `o` may not be a List.  I think we need to check not only `instanceof 
Object[]` but all of the other primitive array types as well.  Probably 
something like this?
    
    ```Java
    for (Object o: selectedFields) {
        if (o instanceof Object[]) {
            out.putInt(Arrays.deepHashCode((Object[])o));
        } else if (o instanceof byte[]) {
            out.putInt(Arrays.hashCode((byte[]) o));
        } else if (o instanceof short[]) {
            out.putInt(Arrays.hashCode((short[]) o));
        } else if (o instanceof int[]) {
            out.putInt(Arrays.hashCode((int[]) o));
        } else if (o instanceof long[]) {
            out.putInt(Arrays.hashCode((long[]) o));
        } else if (o instanceof char[]) {
            out.putInt(Arrays.hashCode((char[]) o));
        } else if (o instanceof float[]) {
            out.putInt(Arrays.hashCode((float[]) o));
        } else if (o instanceof double[]) {
            out.putInt(Arrays.hashCode((double[]) o));
        } else if (o instanceof boolean[]) {
            out.putInt(Arrays.hashCode((boolean[]) o));
        } else if (o != null) {
            out.putInt(o.hashCode());
        } else {
          out.putInt(0);
        }
    ```
    
    This seems to be what 
[Arrays#deepHashCode](http://grepcode.com/file/repository.grepcode.com/java/root/jdk/openjdk/6-b14/java/util/Arrays.java#3709)
 does.
    
    What do you think?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---

Reply via email to