sharanggupta opened a new pull request, #4045: URL: https://github.com/apache/avro/pull/4045
## What is the purpose of the change Fixes [AVRO-4354](https://issues.apache.org/jira/browse/AVRO-4354): since 1.12.1 (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()`. A decoded record holds `GenericData.Array` and `Utf8` values while a built record holds `java.util.List` and `String` values, so: - `equals()` throws `AvroRuntimeException: Unknown datum type java.util.UUID` (or `LocalDate`, `Instant`, ...) for a decoded array whose elements contain a union with a logical type, because `GenericData.Array.equals()` compares through the global `GenericData`, which has no logical type conversions. It is also asymmetric: `List.equals(Array)` compares element by element while `Array.equals(List)` goes through the model, so for string elements one direction sees `String` vs `Utf8` and returns `false` (AVRO-4198). - `hashCode()` of a record holding a `String` differs from the same record after a round trip, which holds a `Utf8`, although `equals()` treats them as equal, so `HashSet`/`HashMap` lookups fail. The fix keeps the fast paths of AVRO-3527 for primitive, `CharSequence`, record, enum, fixed, bytes and converted logical-type fields, and changes the generated code where the runtime type of the value is not under the control of the generated class: - array, map and non-nullable union fields are compared with `MODEL$.compare(this.x, other.x, fieldSchema, true)` and hashed with `MODEL$.hashCode(this.x, fieldSchema)`: the `SpecificData` of the generated class knows the logical type conversions and applies the Avro equality rules (`String`/`Utf8`, `List`/`GenericData.Array`, map keys) symmetrically, as `SpecificRecordBase.equals()`/`hashCode()` did before 1.12.1 for the whole record; - `CharSequence` fields are hashed with the new `Utf8.hashCodeFor(CharSequence)`, which computes the hash code of the UTF-8 encoding of the sequence without encoding it, so that it matches `Utf8.hashCode()` for the same content and `Utf8.compareSequences()`, used by the generated `equals()`. The result is the hash that `SpecificData.hashCode()` computes (used by `GenericData.HashCodeCalculator` now too, which previously allocated a `Utf8` per `String`), so records generated by this compiler, records generated before 1.12.1 and `GenericRecord`s with the same content keep the same hash code; only records generated by 1.12.1/1.12.2 that hold `String`s change hash code, which they must, to honour the `equals()` contract. - `GenericData.compare(Object, Object, Schema, boolean)` (and the `SpecificData`/`ReflectData` overrides) is now public so that generated code can call it; the three-argument `compare()` was already public. - `GenericData.hashCode(Object, Schema)` gains a `MAP` case: it used to fall back to `Map.hashCode()`, whose result depends on whether the keys and values are `String`s or `Utf8`s, while `compareMaps()` treats them as equal. Map entries are now hashed order-independently with the UTF-8 hash of the key and the schema hash of the value; for `Utf8`-keyed maps with `Utf8`, numeric or record values the result is unchanged. As for `Utf8.compareSequences()` in 1.12.1, code generated by this compiler needs this runtime version. Not addressed here, as they look like separate decisions: `GenericData.Array.equals()` called directly with logical type elements (AVRO-4036), and a compiler/plugin option to disable the generation of `equals()`/`hashCode()` (item 4 of the ticket). Happy to add the option in a follow-up if wanted. ## Verifying this change This change added tests and can be verified as follows: - `TestGeneratedEqualsAndHashCode` in the compiler module, on a new test schema `generated_equals.avsc` compiled at build time like the other `SchemaTask` schemas: array of records with a nullable `uuid`, array of nullable `date`, array of strings, nullable array of strings, map of strings, `CharSequence` fields with a `HashSet` lookup, and generated `hashCode()` equal to `SpecificData.hashCode()`. On `main`, `arrayOfRecordsWithNullableLogicalType` and `arrayOfNullableLogicalType` error with `AvroRuntimeException: Unknown datum type java.util.UUID` / `java.time.LocalDate`, `arrayOfStrings`, `nullableArrayOfStrings` and `mapOfStrings` fail on `after.equals(before)`, and the two hash tests fail with different hash codes (4066 vs 5058 for `EqualsNamed("a", "b")`). - `TestUtf8.hashCodeForMatchesUtf8HashCode` and `...ForRandomStrings`: `Utf8.hashCodeFor()` equals `new Utf8(s).hashCode()` for ASCII, 2-, 3- and 4-byte characters, unpaired surrogates and 1000 random strings. - `TestGenericData.testMapHashCodeIsConsistentWithEquals`: a `String`-keyed and a `Utf8`-keyed map with the same content compare equal and hash the same, and the `Utf8` map keeps its previous hash code. - Locally (JDK 21, build cache disabled, `spotless:apply` applied): `avro` (3463 tests), `compiler` (99), `ipc` (238), `maven-plugin` (15) and the `integration-test` codegen modules pass with `verify`. ## Documentation - Does this pull request introduce a new feature? no - If yes, how is the feature documented? not applicable (`Utf8.hashCodeFor()` and the template helpers have JavaDoc) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
