[
https://issues.apache.org/jira/browse/BEAM-5866?focusedWorklogId=159871&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-159871
]
ASF GitHub Bot logged work on BEAM-5866:
----------------------------------------
Author: ASF GitHub Bot
Created on: 29/Oct/18 06:32
Start Date: 29/Oct/18 06:32
Worklog Time Spent: 10m
Work Description: amaliujia commented on a change in pull request #6845:
[BEAM-5866] Fix `Row#equals`
URL: https://github.com/apache/beam/pull/6845#discussion_r228811170
##########
File path: sdks/java/core/src/main/java/org/apache/beam/sdk/values/Row.java
##########
@@ -346,13 +346,126 @@ public boolean equals(Object o) {
return false;
}
Row other = (Row) o;
- return Objects.equals(getSchema(), other.getSchema())
- && Objects.deepEquals(getValues().toArray(),
other.getValues().toArray());
+
+ if (!Objects.equals(getSchema(), other.getSchema())) {
+ return false;
+ }
+
+ for (int i = 0; i < getFieldCount(); i++) {
+ if (!Equals.deepEquals(getValue(i), other.getValue(i),
getSchema().getField(i).getType())) {
+ return false;
+ }
+ }
+
+ return true;
}
@Override
public int hashCode() {
- return Arrays.deepHashCode(new Object[] {getSchema(),
getValues().toArray()});
+ int h = 1;
+ for (int i = 0; i < getFieldCount(); i++) {
+ h = 31 * h + Equals.deepHashCode(getValue(i),
getSchema().getField(i).getType());
+ }
+
+ return h;
+ }
+
+ static class Equals {
+ static boolean deepEquals(Object a, Object b, Schema.FieldType fieldType) {
+ if (fieldType.getTypeName() == Schema.TypeName.BYTES) {
+ return Arrays.equals((byte[]) a, (byte[]) b);
+ } else if (fieldType.getTypeName() == Schema.TypeName.ARRAY) {
+ return deepEqualsForList(
+ (List<Object>) a, (List<Object>) b,
fieldType.getCollectionElementType());
+ } else if (fieldType.getTypeName() == Schema.TypeName.MAP) {
+ return deepEqualsForMap(
+ (Map<Object, Object>) a, (Map<Object, Object>) b,
fieldType.getMapValueType());
+ } else {
+ return Objects.equals(a, b);
+ }
+ }
+
+ static int deepHashCode(Object a, Schema.FieldType fieldType) {
+ if (fieldType.getTypeName() == Schema.TypeName.BYTES) {
+ return Arrays.hashCode((byte[]) a);
+ } else if (fieldType.getTypeName() == Schema.TypeName.ARRAY) {
+ return deepHashCodeForList((List<Object>) a,
fieldType.getCollectionElementType());
+ } else if (fieldType.getTypeName() == Schema.TypeName.MAP) {
+ return deepHashCodeForMap(
+ (Map<Object, Object>) a, fieldType.getMapKeyType(),
fieldType.getMapValueType());
+ } else {
+ return Objects.hashCode(a);
+ }
+ }
+
+ static <K, V> boolean deepEqualsForMap(Map<K, V> a, Map<K, V> b,
Schema.FieldType valueType) {
+ if (a == b) {
+ return true;
+ }
+
+ if (a.size() != b.size()) {
+ return false;
+ }
+
+ for (Map.Entry<K, V> e : a.entrySet()) {
+ K key = e.getKey();
+ V value = e.getValue();
+ V otherValue = b.get(key);
Review comment:
At this moment, I don't have more insights than others. Just an idea about
`Map<byte[], ?>`.
Usually `byte[]` should not be the key of `Map` due to hashcode and quality
of array is based object reference but not value based. So I think for `Map`
with `byte[]` as its key, and for any instance of it, the instance is unique.
So it is ok in this deepEqual, it does not compare the value of byte array.
From the nature of implementation of `Map<byte[], ?>`, there is no need to
compare if two instances are born to be unique.
----------------------------------------------------------------
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]
Issue Time Tracking
-------------------
Worklog Id: (was: 159871)
Time Spent: 5h (was: 4h 50m)
> RowCoder doesn't implement structuralValue
> ------------------------------------------
>
> Key: BEAM-5866
> URL: https://issues.apache.org/jira/browse/BEAM-5866
> Project: Beam
> Issue Type: Bug
> Components: sdk-java-core
> Reporter: Gleb Kanterov
> Assignee: Gleb Kanterov
> Priority: Major
> Time Spent: 5h
> Remaining Estimate: 0h
>
> These two properties fail for RowCoder with `BYTES` field, or `Map<BYTES, ?>`
> field.
> {code}
> public static <T> void testConsistentWithEquals(Coder<T> coder, T example) {
> assumeTrue(coder.consistentWithEquals());
> byte[] bytes = encodeBytes(coder, example);
> // even if the coder is non-deterministic, if the encoded bytes match,
> // coder is consistent with equals, decoded values must be equal
> T out0 = decodeBytes(coder, bytes);
> T out1 = decodeBytes(coder, bytes);
> assertEquals("If the encoded bytes match, decoded values must be equal",
> out0, out1);
> assertEquals(
> "If two values are equal, their hash codes must be equal",
> out0.hashCode(),
> out1.hashCode());
> }
> public static <T> void testStructuralValueConsistentWithEquals(Coder<T>
> coder, T example) {
> byte[] bytes = encodeBytes(coder, example);
> // even if coder is non-deterministic, if the encoded bytes match,
> // structural values must be equal
> Object out0 = coder.structuralValue(decodeBytes(coder, bytes));
> Object out1 = coder.structuralValue(decodeBytes(coder, bytes));
> assertEquals("If the encoded bytes match, structural values must be
> equal", out0, out1);
> assertEquals(
> "If two values are equal, their hash codes must be equal",
> out0.hashCode(),
> out1.hashCode());
> }
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
