Move Avro schema interning inside HoodieSchema once it decouples from Avro
- Dominant language
- Java
- Stars
- 6.2k
- Forks
- 2.5k
- Avg merge
- 2d 8h
- Merged PRs (30d)
- 111
Description
cc @danny0405
Follow-up to #18967, requested in https://github.com/apache/hudi/pull/18967#issuecomment-4839055903.
### Describe the problem
#18967 canonicalizes the Avro `Schema` -> `HoodieSchema` conversion through an explicit `HoodieAvroSchemaCache.intern(...)` call at each per-record call site. The review preference was to cache inside `HoodieSchema` itself (`fromAvroSchema` returning the canonical instance) so call sites stay oblivious, but that was reverted during the PR because it breaks `HoodieSchemaCompatibilityChecker`:
- The checker memoizes on a `ReaderWriter` key that compares schemas by reference (`System.identityHashCode` / `==`) and relies on each schema occurrence being a distinct instance: sibling subschemas of the same shape get distinct memo entries (one reported location each), and revisiting the same instance mid-computation is what signals genuine recursion.
- With interning inside `fromAvroSchema`, structurally-equal subschemas at different field paths collapse into one canonical instance. The first `(reader, writer)` pair is computed and memoized; later occurrences hit the memo and never record their own locations, so incompatibilities are under-reported. `TestHoodieSchemaUtils#testIllegalPromotionsBetweenPrimitives` fails, reporting only `rec.simpleField` instead of all four paths. Full analysis: https://github.com/apache/hudi/pull/18967#discussion_r3426595575
So today the interning stays at the call sites (where instance identity does not matter) and `fromAvroSchema` still allocates a fresh wrapper per call, including sub-schema navigation (`getElementType`, `getValueType`, union branches, `HoodieSchemaField` construction).
### Proposed fix
When `HoodieSchema` is decoupled from Avro (conversion happens once at the engine boundary instead of call sites wrapping live Avro `Schema` instances), fold canonicalization into `HoodieSchema` and remove the call-site interning:
1. Rework `HoodieSchemaCompatibilityChecker` so correctness does not depend on per-occurrence instance identity: key the memo / location bookkeeping on the traversal path rather than object identity, while preserving recursion detection and the guard against exponential recomputation on shared-subschema DAGs. This is the actual blocker and can land independently, ahead of the decoupling. (The checker is ported from Avro `SchemaCompatibility`, which bakes in the same assumption.)
2. Make the conversion entry point return canonical instances directly (today `fromAvroSchema`; post-decoupling, the boundary converter), including sub-schema navigation.
3. Drop the explicit `HoodieAvroSchemaCache.intern(...)` calls and retire `HoodieAvroSchemaCache` (or fold it into `HoodieSchema` as an implementation detail). Current sites: `AvroRecordContext#getFieldValueFromIndexedRecord`, `SparkFileFormatInternalRecordContext#convertAvroRecord`, `FlinkRecordContext#convertAvroRecord`, `RealtimeCompactedRecordReader#mergeRecord`, `HoodieAvroUtils#getRecordColumnValues`, `HoodieJsonPayload#getInsertValue`, and the `ExpressionPayload` evaluator / deserializer / serializer / joinRecords paths.
4. Revisit the cache split: post-decoupling, canonicalization should key on `HoodieSchema` value equality alone (`HoodieSchemaCache`); the Avro-identity `weakKeys` view (`HoodieAvroSchemaCache`) is only meaningful while call sites hand back live Avro `Schema` instances.
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.