From 9b4b14a2959365a490e1c7d4383347188d967210 Mon Sep 17 00:00:00 2001 From: Sharang Gupta Date: Wed, 14 Oct 2026 10:24:05 +0100 Subject: [PATCH 1/2] AVRO-4354: [java] Fix generated equals and hashCode semantics Since AVRO-3527 the generated equals() compares array, map and union fields with java.util.Objects.equals(), and the generated hashCode() hashes CharSequence fields with their own hashCode(). Decoded records hold GenericData.Array and Utf8 values while built records hold java.util.List and String values, so: - equals() throws "Unknown datum type" for a decoded array whose elements contain a union with a logical type, because GenericData.Array.equals() uses the global GenericData, which has no logical type conversions, and List.equals(Array) is not symmetric with Array.equals(List) for string elements; - hashCode() differs for a String and a Utf8 with the same content, although equals() treats them as equal. Array, map and non-nullable union fields are now compared and hashed through the SpecificData model of the generated class, which knows the conversions and the Avro equality rules, and CharSequence fields are hashed with the new Utf8.hashCodeFor(), the hash code of the UTF-8 encoding of the sequence, so that the generated hashCode() agrees with equals() and with SpecificData.hashCode(). The four-argument GenericData.compare() is public, so that generated code can call it. --- .../org/apache/avro/generic/GenericData.java | 15 ++- .../org/apache/avro/reflect/ReflectData.java | 2 +- .../apache/avro/specific/SpecificData.java | 2 +- .../main/java/org/apache/avro/util/Utf8.java | 44 +++++++ .../apache/avro/generic/TestGenericData.java | 19 +++ .../java/org/apache/avro/util/TestUtf8.java | 29 ++++ lang/java/compiler/pom.xml | 1 + .../compiler/specific/SpecificCompiler.java | 39 ++++++ .../specific/templates/java/classic/record.vm | 4 +- .../TestGeneratedEqualsAndHashCode.java | 124 ++++++++++++++++++ .../src/test/resources/generated_equals.avsc | 53 ++++++++ 11 files changed, 327 insertions(+), 5 deletions(-) create mode 100644 lang/java/compiler/src/test/java/org/apache/avro/specific/TestGeneratedEqualsAndHashCode.java create mode 100644 lang/java/compiler/src/test/resources/generated_equals.avsc diff --git a/lang/java/avro/src/main/java/org/apache/avro/generic/GenericData.java b/lang/java/avro/src/main/java/org/apache/avro/generic/GenericData.java index ebbac9a3228..099e920ecde 100644 --- a/lang/java/avro/src/main/java/org/apache/avro/generic/GenericData.java +++ b/lang/java/avro/src/main/java/org/apache/avro/generic/GenericData.java @@ -1173,6 +1173,17 @@ public int hashCode(Object o, Schema s) { currentHashCode = this.hashCodeAdd(e, elementType); } return currentHashCode; + case MAP: + // Consistent with compareMaps(): independent of the entry order and of + // the String or Utf8 implementation of the keys and values. + int mapHashCode = 0; + Schema valueType = s.getValueType(); + for (Map.Entry entry : ((Map) o).entrySet()) { + Object key = entry.getKey(); + int keyHashCode = Utf8.hashCodeFor(key instanceof CharSequence ? (CharSequence) key : key.toString()); + mapHashCode += keyHashCode ^ new HashCodeCalculator().hashCode(entry.getValue(), valueType); + } + return mapHashCode; case UNION: return hashCode(o, s.getTypes().get(GenericData.this.resolveUnion(s, o))); case ENUM: @@ -1180,7 +1191,7 @@ public int hashCode(Object o, Schema s) { case NULL: return 0; case STRING: - return (o instanceof Utf8 ? o : new Utf8(o.toString())).hashCode(); + return Utf8.hashCodeFor(o instanceof CharSequence ? (CharSequence) o : o.toString()); default: return o.hashCode(); } @@ -1275,7 +1286,7 @@ protected int compareMaps(final Map m1, final Map m2) { * for order. */ @SuppressWarnings(value = "unchecked") - protected int compare(Object o1, Object o2, Schema s, boolean equals) { + public int compare(Object o1, Object o2, Schema s, boolean equals) { if (o1 == o2) return 0; switch (s.getType()) { diff --git a/lang/java/avro/src/main/java/org/apache/avro/reflect/ReflectData.java b/lang/java/avro/src/main/java/org/apache/avro/reflect/ReflectData.java index 61e070b0525..317fff17b03 100644 --- a/lang/java/avro/src/main/java/org/apache/avro/reflect/ReflectData.java +++ b/lang/java/avro/src/main/java/org/apache/avro/reflect/ReflectData.java @@ -980,7 +980,7 @@ private Schema getSchema(Type type, Map names) { } @Override - protected int compare(Object o1, Object o2, Schema s, boolean equals) { + public int compare(Object o1, Object o2, Schema s, boolean equals) { switch (s.getType()) { case ARRAY: if (!o1.getClass().isArray()) diff --git a/lang/java/avro/src/main/java/org/apache/avro/specific/SpecificData.java b/lang/java/avro/src/main/java/org/apache/avro/specific/SpecificData.java index dbed69d79bb..ccd3aedd4e5 100644 --- a/lang/java/avro/src/main/java/org/apache/avro/specific/SpecificData.java +++ b/lang/java/avro/src/main/java/org/apache/avro/specific/SpecificData.java @@ -585,7 +585,7 @@ public Protocol getProtocol(Class iface) { } @Override - protected int compare(Object o1, Object o2, Schema s, boolean eq) { + public int compare(Object o1, Object o2, Schema s, boolean eq) { switch (s.getType()) { case ENUM: if (o1 instanceof Enum) diff --git a/lang/java/avro/src/main/java/org/apache/avro/util/Utf8.java b/lang/java/avro/src/main/java/org/apache/avro/util/Utf8.java index a5c4ece29d7..81e06d0c9b0 100644 --- a/lang/java/avro/src/main/java/org/apache/avro/util/Utf8.java +++ b/lang/java/avro/src/main/java/org/apache/avro/util/Utf8.java @@ -232,6 +232,50 @@ public void readExternal(ObjectInput in) throws IOException, ClassNotFoundExcept in.readFully(bytes); } + /** + * Returns the hash code of the UTF-8 encoding of the given character sequence, + * that is the hash code of a {@link Utf8} with the same content, without + * encoding the sequence. A {@link String} and a {@link Utf8} with the same + * content, which are equal according to + * {@link #compareSequences(CharSequence, CharSequence)}, therefore have the + * same hash code. + * + * @param cs the character sequence to hash + * @return the hash code of the UTF-8 encoding of the character sequence + */ + public static int hashCodeFor(CharSequence cs) { + if (cs instanceof Utf8) { + return cs.hashCode(); + } + int h = 1; + int length = cs.length(); + for (int i = 0; i < length; i++) { + char c = cs.charAt(i); + if (c < 0x80) { + h = 31 * h + c; + } else if (c < 0x800) { + h = 31 * h + (byte) (0xC0 | (c >> 6)); + h = 31 * h + (byte) (0x80 | (c & 0x3F)); + } else if (Character.isSurrogate(c)) { + if (Character.isHighSurrogate(c) && i + 1 < length && Character.isLowSurrogate(cs.charAt(i + 1))) { + int codePoint = Character.toCodePoint(c, cs.charAt(++i)); + h = 31 * h + (byte) (0xF0 | (codePoint >> 18)); + h = 31 * h + (byte) (0x80 | ((codePoint >> 12) & 0x3F)); + h = 31 * h + (byte) (0x80 | ((codePoint >> 6) & 0x3F)); + h = 31 * h + (byte) (0x80 | (codePoint & 0x3F)); + } else { + // An unpaired surrogate is encoded as the replacement character. + h = 31 * h + '?'; + } + } else { + h = 31 * h + (byte) (0xE0 | (c >> 12)); + h = 31 * h + (byte) (0x80 | ((c >> 6) & 0x3F)); + h = 31 * h + (byte) (0x80 | (c & 0x3F)); + } + } + return h; + } + public static int compareSequences(CharSequence cs1, CharSequence cs2) { if (cs1 == cs2) { return 0; diff --git a/lang/java/avro/src/test/java/org/apache/avro/generic/TestGenericData.java b/lang/java/avro/src/test/java/org/apache/avro/generic/TestGenericData.java index 3a563725ef5..a48196e1aca 100644 --- a/lang/java/avro/src/test/java/org/apache/avro/generic/TestGenericData.java +++ b/lang/java/avro/src/test/java/org/apache/avro/generic/TestGenericData.java @@ -136,6 +136,25 @@ void testHashCode() { GenericData.get().hashCode(r, schema); } + /** AVRO-4354: the hash code of a map must be consistent with compareMaps(). */ + @Test + void testMapHashCodeIsConsistentWithEquals() { + Schema s = Schema.createMap(Schema.create(Type.STRING)); + Map strings = new HashMap<>(); + strings.put("a", "x"); + strings.put("b", "y"); + Map utf8s = new HashMap<>(); + utf8s.put(new Utf8("b"), new Utf8("y")); + utf8s.put(new Utf8("a"), new Utf8("x")); + Map other = new HashMap<>(); + other.put(new Utf8("a"), new Utf8("z")); + + assertEquals(0, GenericData.get().compare(strings, utf8s, s, true)); + assertEquals(GenericData.get().hashCode(utf8s, s), GenericData.get().hashCode(strings, s)); + assertEquals(utf8s.hashCode(), GenericData.get().hashCode(utf8s, s)); + assertNotEquals(GenericData.get().hashCode(other, s), GenericData.get().hashCode(strings, s)); + } + @Test void testEquals() { Schema s = recordSchema(); diff --git a/lang/java/avro/src/test/java/org/apache/avro/util/TestUtf8.java b/lang/java/avro/src/test/java/org/apache/avro/util/TestUtf8.java index 3e36d9a0214..ede8295efbe 100644 --- a/lang/java/avro/src/test/java/org/apache/avro/util/TestUtf8.java +++ b/lang/java/avro/src/test/java/org/apache/avro/util/TestUtf8.java @@ -27,6 +27,7 @@ import java.io.ObjectInputStream; import java.io.ObjectOutputStream; import java.nio.charset.StandardCharsets; +import java.util.Random; import org.apache.avro.SystemLimitException; import org.apache.avro.TestSystemLimitException; @@ -162,4 +163,32 @@ void serialization() throws IOException, ClassNotFoundException { } } + @Test + void hashCodeForMatchesUtf8HashCode() { + String[] samples = { "", "a", "abc", "0123456789", "caf\u00e9", "\u20ac uro", "\u4e2d\u6587\u5b57\u7b26", + "smile \ud83d\ude00", "\ud83d\ude00\ud83d\ude00\ud83d\ude00", "lone high \ud83d", "lone low \ude00 x", "\ud83d", + "\ude00", "high then ascii \ud83da", "\ud83d\ud83d\ude00" }; + for (String sample : samples) { + int expected = new Utf8(sample).hashCode(); + assertEquals(expected, Utf8.hashCodeFor(sample), sample); + assertEquals(expected, Utf8.hashCodeFor(new StringBuilder(sample)), sample); + assertEquals(expected, Utf8.hashCodeFor(new Utf8(sample)), sample); + } + } + + @Test + void hashCodeForMatchesUtf8HashCodeForRandomStrings() { + Random random = new Random(4354); + for (int i = 0; i < 1000; i++) { + StringBuilder sb = new StringBuilder(); + int length = random.nextInt(40); + for (int j = 0; j < length; j++) { + // Includes unpaired surrogates. + sb.appendCodePoint(random.nextInt(Character.MAX_CODE_POINT + 1)); + } + String sample = sb.toString(); + assertEquals(new Utf8(sample).hashCode(), Utf8.hashCodeFor(sample), sample); + } + } + } diff --git a/lang/java/compiler/pom.xml b/lang/java/compiler/pom.xml index be3163cac0a..6d20ea3e1ab 100644 --- a/lang/java/compiler/pom.xml +++ b/lang/java/compiler/pom.xml @@ -138,6 +138,7 @@ ${project.basedir}/src/test/resources/full_record_v1.avsc ${project.basedir}/src/test/resources/full_record_v2.avsc ${project.basedir}/src/test/resources/regression_error_field_in_record.avsc + ${project.basedir}/src/test/resources/generated_equals.avsc ${project.basedir}/target/generated-test-sources/javacc diff --git a/lang/java/compiler/src/main/java/org/apache/avro/compiler/specific/SpecificCompiler.java b/lang/java/compiler/src/main/java/org/apache/avro/compiler/specific/SpecificCompiler.java index 32a7d97cca0..085e87ce791 100644 --- a/lang/java/compiler/src/main/java/org/apache/avro/compiler/specific/SpecificCompiler.java +++ b/lang/java/compiler/src/main/java/org/apache/avro/compiler/specific/SpecificCompiler.java @@ -1202,6 +1202,41 @@ public boolean isPrimitiveType(Schema schema) { return !isUnboxedJavaTypeNullable(schema) && getConvertedLogicalType(schema) == null; } + /** + * Utility for template use. Returns whether the values of a field with the + * given schema must be compared and hashed through the {@code SpecificData} + * model of the generated class, instead of {@code Object.equals()} and + * {@code Object.hashCode()}. That is the case for arrays and maps, whose + * runtime types differ between built and decoded records (for example + * {@code java.util.List} and {@code GenericData.Array}, or {@code String} and + * {@code Utf8} elements) and whose elements may need the logical type + * conversions of the model, and for unions that are not a nullable type. + */ + public boolean requiresModelComparison(Schema schema) { + switch (schema.getType()) { + case ARRAY: + case MAP: + return true; + case UNION: + List types = schema.getTypes(); + boolean nullable = types.size() == 2 && types.contains(NULL_SCHEMA); + return !nullable || types.stream().anyMatch(this::requiresModelComparison); + default: + return false; + } + } + + /** + * Utility for template use. Returns the expression computing the hash code of + * the given field, consistent with the generated {@code equals()}. + */ + public String hashCodeFor(Field field, String name) { + if (requiresModelComparison(field.schema())) { + return "MODEL$.hashCode(" + name + ", SCHEMA$.getFields().get(" + field.pos() + ").schema())"; + } + return hashCodeFor(field.schema(), name); + } + public String hashCodeFor(Schema schema, String name) { switch (javaUnbox(schema, false)) { case "int": @@ -1226,6 +1261,10 @@ public String hashCodeFor(Schema schema, String name) { + ").ordinal() : " + name + ".hashCode())"; } } + if ("java.lang.CharSequence".equals(javaType(schema))) { + // Consistent with Utf8.compareSequences(), used by the generated equals(). + return "(" + name + " == null ? 0 : Utf8.hashCodeFor(" + name + "))"; + } return "(" + name + " == null ? 0 : " + name + ".hashCode())"; } } diff --git a/lang/java/compiler/src/main/velocity/org/apache/avro/compiler/specific/templates/java/classic/record.vm b/lang/java/compiler/src/main/velocity/org/apache/avro/compiler/specific/templates/java/classic/record.vm index cd89d7dc609..eaf0476ce15 100755 --- a/lang/java/compiler/src/main/velocity/org/apache/avro/compiler/specific/templates/java/classic/record.vm +++ b/lang/java/compiler/src/main/velocity/org/apache/avro/compiler/specific/templates/java/classic/record.vm @@ -625,7 +625,7 @@ public class ${this.mangleTypeIdentifier($schema.getName())} extends ${this.getS #foreach ($field in $schema.getFields()) #if (!${this.ignoredField($field)}) #set ($n = ${this.mangle($field.name(), $schema.isError())}) - result = 31 * result + ${this.hashCodeFor($field.schema(), "this." + $n)}; + result = 31 * result + ${this.hashCodeFor($field, "this." + $n)}; #end #end return result; @@ -648,6 +648,8 @@ public class ${this.mangleTypeIdentifier($schema.getName())} extends ${this.getS if (this.$n != other.$n) { #elseif (${this.javaType($field.schema()).equals("java.lang.CharSequence")}) if (Utf8.compareSequences(this.$n, other.$n) != 0) { +#elseif (${this.requiresModelComparison($s)}) + if (MODEL$.compare(this.$n, other.$n, SCHEMA$.getFields().get(${field.pos()}).schema(), true) != 0) { #else if (!java.util.Objects.equals(this.$n, other.$n)) { #end diff --git a/lang/java/compiler/src/test/java/org/apache/avro/specific/TestGeneratedEqualsAndHashCode.java b/lang/java/compiler/src/test/java/org/apache/avro/specific/TestGeneratedEqualsAndHashCode.java new file mode 100644 index 00000000000..97a9fc5ab79 --- /dev/null +++ b/lang/java/compiler/src/test/java/org/apache/avro/specific/TestGeneratedEqualsAndHashCode.java @@ -0,0 +1,124 @@ +/* + * Copyright 2017 The Apache Software Foundation. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.avro.specific; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.io.IOException; +import java.time.LocalDate; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.UUID; + +import org.apache.avro.specific.test.EqualsLine; +import org.apache.avro.specific.test.EqualsNamed; +import org.apache.avro.specific.test.EqualsOrder; +import org.junit.jupiter.api.Test; + +/** + * AVRO-4354: the generated {@code equals()} must not depend on the runtime + * types of collections and strings, and the generated {@code hashCode()} must + * be consistent with it. + */ +public class TestGeneratedEqualsAndHashCode { + + private static final UUID ID = UUID.fromString("de5714a7-0000-0000-0000-000000000001"); + + private static EqualsOrder roundTrip(EqualsOrder order) throws IOException { + return EqualsOrder.getDecoder().decode(EqualsOrder.getEncoder().encode(order)); + } + + private static EqualsNamed roundTrip(EqualsNamed named) throws IOException { + return EqualsNamed.getDecoder().decode(EqualsNamed.getEncoder().encode(named)); + } + + private static void assertEqualBothWays(Object before, Object after) { + assertEquals(before, after); + assertEquals(after, before); + assertEquals(before.hashCode(), after.hashCode()); + } + + @Test + void arrayOfRecordsWithNullableLogicalType() throws IOException { + EqualsOrder before = EqualsOrder.newBuilder().setLines(List.of(EqualsLine.newBuilder().setLineId(ID).build())) + .build(); + EqualsOrder after = roundTrip(before); + + assertEqualBothWays(before, after); + assertEqualBothWays(after, roundTrip(before)); + } + + @Test + void arrayOfNullableLogicalType() throws IOException { + EqualsOrder before = EqualsOrder.newBuilder().setDates(List.of(LocalDate.of(2026, 9, 28))).build(); + EqualsOrder after = roundTrip(before); + + assertEqualBothWays(before, after); + } + + @Test + void arrayOfStrings() throws IOException { + EqualsOrder before = EqualsOrder.newBuilder().setLabels(List.of("a", "b")).build(); + EqualsOrder after = roundTrip(before); + + assertEqualBothWays(before, after); + assertNotEquals(before, EqualsOrder.newBuilder().setLabels(List.of("a")).build()); + } + + @Test + void nullableArrayOfStrings() throws IOException { + EqualsOrder before = EqualsOrder.newBuilder().setOptionalLabels(List.of("a")).build(); + EqualsOrder after = roundTrip(before); + + assertEqualBothWays(before, after); + assertNotEquals(before, EqualsOrder.newBuilder().build()); + } + + @Test + void mapOfStrings() throws IOException { + EqualsOrder before = EqualsOrder.newBuilder().setAttributes(Map.of("k", "v")).build(); + EqualsOrder after = roundTrip(before); + + assertEqualBothWays(before, after); + assertNotEquals(before, EqualsOrder.newBuilder().setAttributes(Map.of("k", "w")).build()); + } + + @Test + void hashCodeIsConsistentWithEqualsForCharSequenceFields() throws IOException { + // Holds java.lang.String values. + EqualsNamed before = EqualsNamed.newBuilder().setName("a").setNickname("b").build(); + // Holds org.apache.avro.util.Utf8 values. + EqualsNamed after = roundTrip(before); + + assertEqualBothWays(before, after); + + Set set = new HashSet<>(); + set.add(before); + assertTrue(set.contains(after)); + } + + @Test + void hashCodeOfGeneratedRecordMatchesSpecificData() throws IOException { + EqualsNamed named = EqualsNamed.newBuilder().setName("a").setNickname("b").build(); + assertEquals(SpecificData.get().hashCode(named, named.getSchema()), named.hashCode()); + assertEquals(named.hashCode(), roundTrip(named).hashCode()); + } +} diff --git a/lang/java/compiler/src/test/resources/generated_equals.avsc b/lang/java/compiler/src/test/resources/generated_equals.avsc new file mode 100644 index 00000000000..84c33c75155 --- /dev/null +++ b/lang/java/compiler/src/test/resources/generated_equals.avsc @@ -0,0 +1,53 @@ +[ + { + "type": "record", + "name": "EqualsOrder", + "namespace": "org.apache.avro.specific.test", + "doc": "AVRO-4354: fields whose generated equals() and hashCode() must follow Avro semantics", + "fields": [ + { + "name": "lines", + "type": { + "type": "array", + "items": { + "type": "record", + "name": "EqualsLine", + "fields": [ + { "name": "line_id", "type": ["null", { "type": "string", "logicalType": "uuid" }], "default": null } + ] + } + }, + "default": [] + }, + { + "name": "dates", + "type": { "type": "array", "items": ["null", { "type": "int", "logicalType": "date" }] }, + "default": [] + }, + { + "name": "labels", + "type": { "type": "array", "items": "string" }, + "default": [] + }, + { + "name": "attributes", + "type": { "type": "map", "values": "string" }, + "default": {} + }, + { + "name": "optionalLabels", + "type": ["null", { "type": "array", "items": "string" }], + "default": null + } + ] + }, + { + "type": "record", + "name": "EqualsNamed", + "namespace": "org.apache.avro.specific.test", + "fields": [ + { "name": "name", "type": "string" }, + { "name": "nickname", "type": ["null", "string"], "default": null } + ] + } +] From 81ef4c0a0bdba2ac51b29ea36096aa165b8c35ba Mon Sep 17 00:00:00 2001 From: Sharang Gupta Date: Thu, 15 Oct 2026 17:24:21 +0100 Subject: [PATCH 2/2] AVRO-4354: [java] Handle null fields and map values, update expected code The generated equals() delegated array, map and non-nullable union fields to SpecificData.compare(), which dereferences arrays and maps, so a record whose array or map field is null (possible through the constructors and setters) threw a NullPointerException where Objects.equals() returned false. Check for null before delegating. GenericData.compare() compared map values with Object.equals(), so a map whose values are arrays or maps had the same problems as array fields: "Unknown datum type" for logical types in unions, and String vs Utf8 asymmetries. Compare the values with the value schema through a new compareMaps(Map, Map, Schema); the two-argument compareMaps() keeps its behaviour. Also regenerate the expected classes of TestSpecificCompilerTool, whose equals() and hashCode() follow the new template. --- .../org/apache/avro/generic/GenericData.java | 26 +++++++++- .../specific/templates/java/classic/record.vm | 3 +- .../TestGeneratedEqualsAndHashCode.java | 49 +++++++++++++++++++ .../src/test/resources/generated_equals.avsc | 20 ++++++++ .../avro/examples/baseball/Player.java | 5 +- .../output/AddExtraOptionalGettersTest.java | 2 +- .../test/compiler/output/NoSettersTest.java | 2 +- .../output/OptionalGettersAllFieldsTest.java | 9 ++-- .../OptionalGettersNullableFieldsTest.java | 14 +++--- .../src/test/compiler/output/Player.java | 9 ++-- 10 files changed, 118 insertions(+), 21 deletions(-) diff --git a/lang/java/avro/src/main/java/org/apache/avro/generic/GenericData.java b/lang/java/avro/src/main/java/org/apache/avro/generic/GenericData.java index 099e920ecde..9d9da15898f 100644 --- a/lang/java/avro/src/main/java/org/apache/avro/generic/GenericData.java +++ b/lang/java/avro/src/main/java/org/apache/avro/generic/GenericData.java @@ -1217,6 +1217,24 @@ public int compare(Object o1, Object o2, Schema s) { } protected int compareMaps(final Map m1, final Map m2) { + return compareMaps(m1, m2, null); + } + + /** + * Compares two maps for equality. With a value schema, the values are compared + * with {@link #compare(Object, Object, Schema, boolean)}, so that the + * conversions of this model and the equality of Avro, such as a {@link Utf8} + * equal to a {@link String} with the same content, also apply to values that + * are arrays or maps. Without one, the values are compared with + * {@link Object#equals(Object)}, treating a {@link Utf8} and a {@link String} + * with the same content as equal. + * + * @param m1 the first map + * @param m2 the second map + * @param valueSchema the schema of the values, or {@code null} + * @return 0 if the maps are equal, 1 otherwise + */ + protected int compareMaps(final Map m1, final Map m2, final Schema valueSchema) { if (m1 == m2) { return 0; } @@ -1259,7 +1277,11 @@ protected int compareMaps(final Map m1, final Map m2) { } } else { final Object value2 = m2.get(lookupKey); - if (value instanceof Utf8 && value2 instanceof String) { + if (valueSchema != null) { + if (value2 == null || compare(value, value2, valueSchema, true) != 0) { + return 1; + } + } else if (value instanceof Utf8 && value2 instanceof String) { if (!value.toString().equals(value2)) { return 1; } @@ -1317,7 +1339,7 @@ public int compare(Object o1, Object o2, Schema s, boolean equals) { return e1.hasNext() ? 1 : (e2.hasNext() ? -1 : 0); case MAP: if (equals) - return compareMaps((Map) o1, (Map) o2); + return compareMaps((Map) o1, (Map) o2, s.getValueType()); throw new AvroRuntimeException("Can't compare maps!"); case UNION: int i1 = resolveUnion(s, o1); diff --git a/lang/java/compiler/src/main/velocity/org/apache/avro/compiler/specific/templates/java/classic/record.vm b/lang/java/compiler/src/main/velocity/org/apache/avro/compiler/specific/templates/java/classic/record.vm index eaf0476ce15..a0aa1e5dac3 100755 --- a/lang/java/compiler/src/main/velocity/org/apache/avro/compiler/specific/templates/java/classic/record.vm +++ b/lang/java/compiler/src/main/velocity/org/apache/avro/compiler/specific/templates/java/classic/record.vm @@ -649,7 +649,8 @@ public class ${this.mangleTypeIdentifier($schema.getName())} extends ${this.getS #elseif (${this.javaType($field.schema()).equals("java.lang.CharSequence")}) if (Utf8.compareSequences(this.$n, other.$n) != 0) { #elseif (${this.requiresModelComparison($s)}) - if (MODEL$.compare(this.$n, other.$n, SCHEMA$.getFields().get(${field.pos()}).schema(), true) != 0) { + if (this.$n == null ? other.$n != null : other.$n == null + || MODEL$.compare(this.$n, other.$n, SCHEMA$.getFields().get(${field.pos()}).schema(), true) != 0) { #else if (!java.util.Objects.equals(this.$n, other.$n)) { #end diff --git a/lang/java/compiler/src/test/java/org/apache/avro/specific/TestGeneratedEqualsAndHashCode.java b/lang/java/compiler/src/test/java/org/apache/avro/specific/TestGeneratedEqualsAndHashCode.java index 97a9fc5ab79..034ee5dfac0 100644 --- a/lang/java/compiler/src/test/java/org/apache/avro/specific/TestGeneratedEqualsAndHashCode.java +++ b/lang/java/compiler/src/test/java/org/apache/avro/specific/TestGeneratedEqualsAndHashCode.java @@ -22,6 +22,7 @@ import java.io.IOException; import java.time.LocalDate; +import java.util.Arrays; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -101,6 +102,54 @@ void mapOfStrings() throws IOException { assertNotEquals(before, EqualsOrder.newBuilder().setAttributes(Map.of("k", "w")).build()); } + @Test + void mapOfArraysOfNullableLogicalType() throws IOException { + EqualsOrder before = EqualsOrder.newBuilder().setIdsByKey(Map.of("k", Arrays.asList(ID, null))).build(); + EqualsOrder after = roundTrip(before); + + assertEqualBothWays(before, after); + assertEqualBothWays(after, roundTrip(before)); + } + + @Test + void mapOfArraysOfStrings() throws IOException { + EqualsOrder before = EqualsOrder.newBuilder().setTagsByKey(Map.of("k", List.of("a", "b"))).build(); + EqualsOrder after = roundTrip(before); + + assertEqualBothWays(before, after); + assertNotEquals(before, EqualsOrder.newBuilder().setTagsByKey(Map.of("k", List.of("a"))).build()); + } + + @Test + void mapOfMapsOfStrings() throws IOException { + EqualsOrder before = EqualsOrder.newBuilder().setNestedAttributes(Map.of("k", Map.of("x", "y"))).build(); + EqualsOrder after = roundTrip(before); + + assertEqualBothWays(before, after); + assertNotEquals(before, EqualsOrder.newBuilder().setNestedAttributes(Map.of("k", Map.of("x", "z"))).build()); + } + + @Test + void unionOfStringAndInt() throws IOException { + EqualsOrder before = EqualsOrder.newBuilder().setChoice("a").build(); + EqualsOrder after = roundTrip(before); + + assertEqualBothWays(before, after); + assertNotEquals(before, EqualsOrder.newBuilder().setChoice(1).build()); + } + + @Test + void nullCollectionFieldsDoNotThrow() { + // The constructor, unlike the builder, leaves array and map fields null. + EqualsOrder unset = new EqualsOrder(); + EqualsOrder defaults = EqualsOrder.newBuilder().build(); + + assertNotEquals(unset, defaults); + assertNotEquals(defaults, unset); + assertEquals(unset, new EqualsOrder()); + assertEquals(unset.hashCode(), new EqualsOrder().hashCode()); + } + @Test void hashCodeIsConsistentWithEqualsForCharSequenceFields() throws IOException { // Holds java.lang.String values. diff --git a/lang/java/compiler/src/test/resources/generated_equals.avsc b/lang/java/compiler/src/test/resources/generated_equals.avsc index 84c33c75155..87105b988cf 100644 --- a/lang/java/compiler/src/test/resources/generated_equals.avsc +++ b/lang/java/compiler/src/test/resources/generated_equals.avsc @@ -38,6 +38,26 @@ "name": "optionalLabels", "type": ["null", { "type": "array", "items": "string" }], "default": null + }, + { + "name": "idsByKey", + "type": { "type": "map", "values": { "type": "array", "items": ["null", { "type": "string", "logicalType": "uuid" }] } }, + "default": {} + }, + { + "name": "tagsByKey", + "type": { "type": "map", "values": { "type": "array", "items": "string" } }, + "default": {} + }, + { + "name": "nestedAttributes", + "type": { "type": "map", "values": { "type": "map", "values": "string" } }, + "default": {} + }, + { + "name": "choice", + "type": ["null", "string", "int"], + "default": null } ] }, diff --git a/lang/java/tools/src/test/compiler/output-string/avro/examples/baseball/Player.java b/lang/java/tools/src/test/compiler/output-string/avro/examples/baseball/Player.java index f95d38ed88e..c3a57cb0cbd 100644 --- a/lang/java/tools/src/test/compiler/output-string/avro/examples/baseball/Player.java +++ b/lang/java/tools/src/test/compiler/output-string/avro/examples/baseball/Player.java @@ -597,7 +597,7 @@ public int hashCode() { result = 31 * result + Integer.hashCode(this.number); result = 31 * result + (this.first_name == null ? 0 : this.first_name.hashCode()); result = 31 * result + (this.last_name == null ? 0 : this.last_name.hashCode()); - result = 31 * result + (this.position == null ? 0 : this.position.hashCode()); + result = 31 * result + MODEL$.hashCode(this.position, SCHEMA$.getFields().get(3).schema()); return result; } @@ -619,7 +619,8 @@ public boolean equals(Object o) { if (!java.util.Objects.equals(this.last_name, other.last_name)) { return false; } - if (!java.util.Objects.equals(this.position, other.position)) { + if (this.position == null ? other.position != null : other.position == null + || MODEL$.compare(this.position, other.position, SCHEMA$.getFields().get(3).schema(), true) != 0) { return false; } return true; diff --git a/lang/java/tools/src/test/compiler/output/AddExtraOptionalGettersTest.java b/lang/java/tools/src/test/compiler/output/AddExtraOptionalGettersTest.java index 8a4f288190a..c0b108b2e6f 100644 --- a/lang/java/tools/src/test/compiler/output/AddExtraOptionalGettersTest.java +++ b/lang/java/tools/src/test/compiler/output/AddExtraOptionalGettersTest.java @@ -435,7 +435,7 @@ public AddExtraOptionalGettersTest build() { @Override public int hashCode() { int result = 1; - result = 31 * result + (this.name == null ? 0 : this.name.hashCode()); + result = 31 * result + (this.name == null ? 0 : Utf8.hashCodeFor(this.name)); result = 31 * result + (this.favorite_number == null ? 0 : this.favorite_number.hashCode()); return result; } diff --git a/lang/java/tools/src/test/compiler/output/NoSettersTest.java b/lang/java/tools/src/test/compiler/output/NoSettersTest.java index 7ff527a92f2..79d597cc976 100644 --- a/lang/java/tools/src/test/compiler/output/NoSettersTest.java +++ b/lang/java/tools/src/test/compiler/output/NoSettersTest.java @@ -393,7 +393,7 @@ public NoSettersTest build() { @Override public int hashCode() { int result = 1; - result = 31 * result + (this.name == null ? 0 : this.name.hashCode()); + result = 31 * result + (this.name == null ? 0 : Utf8.hashCodeFor(this.name)); result = 31 * result + (this.favorite_number == null ? 0 : this.favorite_number.hashCode()); return result; } diff --git a/lang/java/tools/src/test/compiler/output/OptionalGettersAllFieldsTest.java b/lang/java/tools/src/test/compiler/output/OptionalGettersAllFieldsTest.java index 8eea77186e2..49e15cf7d1a 100644 --- a/lang/java/tools/src/test/compiler/output/OptionalGettersAllFieldsTest.java +++ b/lang/java/tools/src/test/compiler/output/OptionalGettersAllFieldsTest.java @@ -498,9 +498,9 @@ public OptionalGettersAllFieldsTest build() { @Override public int hashCode() { int result = 1; - result = 31 * result + (this.name == null ? 0 : this.name.hashCode()); - result = 31 * result + (this.nullable_name == null ? 0 : this.nullable_name.hashCode()); - result = 31 * result + (this.favorite_number == null ? 0 : this.favorite_number.hashCode()); + result = 31 * result + (this.name == null ? 0 : Utf8.hashCodeFor(this.name)); + result = 31 * result + (this.nullable_name == null ? 0 : Utf8.hashCodeFor(this.nullable_name)); + result = 31 * result + MODEL$.hashCode(this.favorite_number, SCHEMA$.getFields().get(2).schema()); result = 31 * result + (this.nullable_favorite_number == null ? 0 : this.nullable_favorite_number.hashCode()); return result; } @@ -520,7 +520,8 @@ public boolean equals(Object o) { if (Utf8.compareSequences(this.nullable_name, other.nullable_name) != 0) { return false; } - if (!java.util.Objects.equals(this.favorite_number, other.favorite_number)) { + if (this.favorite_number == null ? other.favorite_number != null : other.favorite_number == null + || MODEL$.compare(this.favorite_number, other.favorite_number, SCHEMA$.getFields().get(2).schema(), true) != 0) { return false; } if (!java.util.Objects.equals(this.nullable_favorite_number, other.nullable_favorite_number)) { diff --git a/lang/java/tools/src/test/compiler/output/OptionalGettersNullableFieldsTest.java b/lang/java/tools/src/test/compiler/output/OptionalGettersNullableFieldsTest.java index fa19683ac19..3c11bbcce5f 100644 --- a/lang/java/tools/src/test/compiler/output/OptionalGettersNullableFieldsTest.java +++ b/lang/java/tools/src/test/compiler/output/OptionalGettersNullableFieldsTest.java @@ -570,11 +570,11 @@ public OptionalGettersNullableFieldsTest build() { @Override public int hashCode() { int result = 1; - result = 31 * result + (this.name == null ? 0 : this.name.hashCode()); - result = 31 * result + (this.nullable_name == null ? 0 : this.nullable_name.hashCode()); - result = 31 * result + (this.favorite_number == null ? 0 : this.favorite_number.hashCode()); + result = 31 * result + (this.name == null ? 0 : Utf8.hashCodeFor(this.name)); + result = 31 * result + (this.nullable_name == null ? 0 : Utf8.hashCodeFor(this.nullable_name)); + result = 31 * result + MODEL$.hashCode(this.favorite_number, SCHEMA$.getFields().get(2).schema()); result = 31 * result + (this.nullable_favorite_number == null ? 0 : this.nullable_favorite_number.hashCode()); - result = 31 * result + (this.nullable_array == null ? 0 : this.nullable_array.hashCode()); + result = 31 * result + MODEL$.hashCode(this.nullable_array, SCHEMA$.getFields().get(4).schema()); return result; } @@ -593,13 +593,15 @@ public boolean equals(Object o) { if (Utf8.compareSequences(this.nullable_name, other.nullable_name) != 0) { return false; } - if (!java.util.Objects.equals(this.favorite_number, other.favorite_number)) { + if (this.favorite_number == null ? other.favorite_number != null : other.favorite_number == null + || MODEL$.compare(this.favorite_number, other.favorite_number, SCHEMA$.getFields().get(2).schema(), true) != 0) { return false; } if (!java.util.Objects.equals(this.nullable_favorite_number, other.nullable_favorite_number)) { return false; } - if (!java.util.Objects.equals(this.nullable_array, other.nullable_array)) { + if (this.nullable_array == null ? other.nullable_array != null : other.nullable_array == null + || MODEL$.compare(this.nullable_array, other.nullable_array, SCHEMA$.getFields().get(4).schema(), true) != 0) { return false; } return true; diff --git a/lang/java/tools/src/test/compiler/output/Player.java b/lang/java/tools/src/test/compiler/output/Player.java index e37b78bc097..2aa6b08cf37 100644 --- a/lang/java/tools/src/test/compiler/output/Player.java +++ b/lang/java/tools/src/test/compiler/output/Player.java @@ -595,9 +595,9 @@ public Player build() { public int hashCode() { int result = 1; result = 31 * result + Integer.hashCode(this.number); - result = 31 * result + (this.first_name == null ? 0 : this.first_name.hashCode()); - result = 31 * result + (this.last_name == null ? 0 : this.last_name.hashCode()); - result = 31 * result + (this.position == null ? 0 : this.position.hashCode()); + result = 31 * result + (this.first_name == null ? 0 : Utf8.hashCodeFor(this.first_name)); + result = 31 * result + (this.last_name == null ? 0 : Utf8.hashCodeFor(this.last_name)); + result = 31 * result + MODEL$.hashCode(this.position, SCHEMA$.getFields().get(3).schema()); return result; } @@ -619,7 +619,8 @@ public boolean equals(Object o) { if (Utf8.compareSequences(this.last_name, other.last_name) != 0) { return false; } - if (!java.util.Objects.equals(this.position, other.position)) { + if (this.position == null ? other.position != null : other.position == null + || MODEL$.compare(this.position, other.position, SCHEMA$.getFields().get(3).schema(), true) != 0) { return false; } return true;