Merge pull request apache#8368: [BEAM-7125] Row throws NPE when compa…

…ring instances with complex null values
mransley · Apr 20, 2019 · f702098 · f702098
2 parents 23276fe + c1c6d29
commit f702098
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 2 deletions.
diff --git a/sdks/java/core/src/main/java/org/apache/beam/sdk/values/Row.java b/sdks/java/core/src/main/java/org/apache/beam/sdk/values/Row.java
@@ -374,7 +374,9 @@ public int hashCode() {
 
  static class Equals {
  static boolean deepEquals(Object a, Object b, Schema.FieldType fieldType) {
- if (fieldType.getTypeName() == TypeName.LOGICAL_TYPE) {
+ if (a == null || b == null) {
+ return a == b;
+ } else if (fieldType.getTypeName() == TypeName.LOGICAL_TYPE) {
  return deepEquals(a, b, fieldType.getLogicalType().getBaseType());
  } else if (fieldType.getTypeName() == Schema.TypeName.BYTES) {
  return Arrays.equals((byte[]) a, (byte[]) b);
@@ -390,7 +392,9 @@ static boolean deepEquals(Object a, Object b, Schema.FieldType fieldType) {
  }
 
  static int deepHashCode(Object a, Schema.FieldType fieldType) {
- if (fieldType.getTypeName() == TypeName.LOGICAL_TYPE) {
+ if (a == null) {
+ return 0;
+ } else if (fieldType.getTypeName() == TypeName.LOGICAL_TYPE) {
  return deepHashCode(a, fieldType.getLogicalType().getBaseType());
  } else if (fieldType.getTypeName() == Schema.TypeName.BYTES) {
  return Arrays.hashCode((byte[]) a);

diff --git a/sdks/java/core/src/test/java/org/apache/beam/sdk/values/RowTest.java b/sdks/java/core/src/test/java/org/apache/beam/sdk/values/RowTest.java
@@ -20,6 +20,7 @@
 import static org.apache.beam.sdk.schemas.Schema.toSchema;
 import static org.apache.beam.sdk.values.Row.toRow;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.assertNull;
 
 import java.math.BigDecimal;
@@ -172,6 +173,21 @@ public void testCreatesArray() {
  assertEquals(data, row.getArray("array"));
  }
 
+ @Test
+ public void testCreatesAndComparesNullArray() {
+ List<Integer> data = null;
+ Schema type =
+ Stream.of(Schema.Field.nullable("array", Schema.FieldType.array(Schema.FieldType.INT32)))
+ .collect(toSchema());
+ Row row = Row.withSchema(type).addArray(data).build();
+ assertEquals(data, row.getArray("array"));
+
+ Row otherNonNull = Row.withSchema(type).addValue(ImmutableList.of(1, 2, 3)).build();
+ Row otherNull = Row.withSchema(type).addValue(null).build();
+ assertNotEquals(otherNonNull, row);
+ assertEquals(otherNull, row);
+ }
+
  @Test
  public void testCreatesArrayWithNullElement() {
  List<Integer> data = Lists.newArrayList(2, null, 5, null);
@@ -253,6 +269,21 @@ public void testCreateMapWithPrimitiveValue() {
  assertEquals(data, row.getMap("map"));
  }
 
+ @Test
+ public void testCreateAndCompareNullMap() {
+ List<Integer> data = null;
+ Schema type =
+ Stream.of(Schema.Field.nullable("map", FieldType.map(FieldType.INT32, FieldType.STRING)))
+ .collect(toSchema());
+ Row row = Row.withSchema(type).addValue(data).build();
+ assertEquals(data, row.getArray("map"));
+
+ Row otherNonNull = Row.withSchema(type).addValue(ImmutableMap.of(1, "value1")).build();
+ Row otherNull = Row.withSchema(type).addValue(null).build();
+ assertNotEquals(otherNonNull, row);
+ assertEquals(otherNull, row);
+ }
+
  @Test
  public void testCreateMapWithNullValue() {
  Map<Integer, String> data = new HashMap();