refactor(schema): schema util contracts, dead API and folds - #19809
Conversation
Reduce the visibility of schema-util members that have no callers outside their own class, so the public surface of HoodieAvroUtils, HoodieAvroWrapperUtils, HoodieSchemaUtils and HoodieSchemaCompatibilityChecker only lists what other code uses. No bodies or signatures change. HoodieAvroUtils: indexedRecordToBytesStream, avroToJsonString, wrapNullable, getNullableValAsString and gteqAvro1_12 become private; convertBytesToFixed and gteqAvro1_10 become package-private (their only callers are same-package tests); makeFieldNonNull, rewriteRecords and gteqAvro1_9 stay public but are marked @VisibleForTesting because their only callers are tests in other modules. HoodieSchemaUtils: hasSmallPrecisionDecimalField becomes package-private; addMetadataColumnTypes stays public and is marked @VisibleForTesting (callers are hudi-hadoop-mr and client-common tests). HoodieAvroWrapperUtils: wrapArray and the 2-arg unwrapAvroValueWrapper become private. HoodieSchemaCompatibilityChecker: READER_WRITER_COMPATIBLE_MESSAGE becomes private. Kept as-is after re-checking callers: convertBytesToBigDecimal(byte[], Decimal) (TestMercifulJsonConverter) and the 5-arg HoodieSchemaUtils.createNewSchemaField with HoodieFieldOrder (ExpressionPayload.mergeSchema in hudi-spark-common). Part of apache#16639; absorbs apache#15908.
Move LocalHoodieSchemaCache from common.util to common.schema, next to the other schema caches (HoodieSchemaCache, HoodieAvroSchemaCache), and rename its factory from getInstance() to create(): it returns a new, empty cache on every call, so the old name suggested a singleton it is not. RecordContext is the only caller. Note: RecordContext is Serializable and holds the cache as a non-transient field, so the Java-serialized form now names the new package. RecordContext is per-task state and is never persisted. Part of apache#16639.
AvroSchemaCache (Schema -> Schema interning) had a single call site: HoodieMetadataPayload interning HoodieMetadataRecord.getClassSchema(), a generated-class static that is already a singleton. Because that was the only intern() call in the repo, the cache was always empty when the static initializer ran and intern() necessarily returned its argument, so the identity check in HoodieMetadataPayload#getInsertValue is unaffected. HoodieAvroSchemaCache (Schema -> HoodieSchema) is the live Avro-keyed cache and stays. Also drop two unused Scala imports of the deleted class and repoint the HoodieInternalRowUtils scaladoc at HoodieAvroSchemaCache. Part of apache#16639.
HoodieSchemaUtils is the home of table-schema transforms, but it carried two value-level helpers that only forwarded to HoodieAvroUtils: - convertValueForSpecificDataTypes(HoodieSchema, Object, boolean) had no production callers (HoodieAvroUtils#getNestedFieldVal calls its own Schema-typed overload) and is deleted; its ten unit tests move to TestHoodieAvroUtils against the Avro overload with the assertions unchanged. - convertBytesToBigDecimal(byte[], HoodieSchema) moves next to the other decimal decoders in HoodieAvroUtils and now calls the (byte[], int, int) overload directly; the HoodieSchemaUtils (byte[], int, int) shim is deleted. DecimalLogicalTypeProcessor and TestJsonKafkaSource switch to the new home. TestDataSourceDefaults passes the raw Avro field schema instead of wrapping it only to unwrap it again. Two tests pin the moved decimal decoder (bytes-backed value; non-DECIMAL schema rejected). Part of apache#16639.
HoodieAvroUtils#getRecordColumnValues took an Avro Schema and immediately re-wrapped it through HoodieAvroSchemaCache.intern to call HoodieRecord#toIndexedRecord(HoodieSchema, ...), while both callers (HoodieAvroRecord#getColumnValues, HoodieTableMetadataUtil) already held a HoodieSchema and unwrapped it with toAvroSchema() just to satisfy the signature. Take HoodieSchema directly, like the sibling getSortColumnValuesWithPartitionPathAndRecordKey already does, and drop the round trip; HoodieAvroUtils no longer needs HoodieAvroSchemaCache. Adds a direct unit test for getRecordColumnValues (there was none). Part of apache#16639.
HoodieSchemas ("Factory class for HoodieSchema") held a single method,
createDeleteLogSchema, and gave contributors a third place to look for
schema helpers next to HoodieSchema's own factories and
HoodieSchemaUtils. Move the method, verbatim, into HoodieSchemaUtils
beside the other well-known Hudi record shapes (getRecordKeySchema,
getRecordKeyPartitionPathSchema) and delete the class. Six call sites
switch to the new home.
Adds unit tests for createDeleteLogSchema (there were none): record
name, record-key field first and non-nullable, ordering field made
nullable with a null default and its doc preserved, unknown ordering
field rejected.
Part of apache#16639.
Write down where schema helpers belong so the next contributor finds the existing one instead of adding a twin (the failure mode apache#16639 describes; see 472bc0c, e3f9342 and apache#19212 for past instances): - HoodieAvroUtils: operations on Avro records and values; raw Schema helpers only in service of those. Lists the union-unwrapping family (unwrapNullable, getActualSchemaFromUnion, AvroSchemaUtils#getNonNullTypeFromUnion, HoodieSchema#getNonNullType, HoodieSchemaUtils#resolveUnionSchema, AvroOrcUtils#getActualSchemaType) with their differing contracts, which apache#19212 kept apart on purpose. - HoodieSchemaUtils: HoodieSchema-typed structural transforms of table schemas and the well-known Hudi record shapes; no value or record operations. Replaces a javadoc that claimed every method delegates to Avro (7 of 32 do) and names the delegations still to be retired. - HoodieSchemaRepair, HoodieAvroWrapperUtils: class javadocs (had none). - HoodieSchemaTypePromotion: the single primitive-widening table used by the projection checker; explains why TIMESTAMP<-LONG and UUID<-STRING are accepted by the compatibility checker but are not projection promotions, and records the decimal fixed-size-parity difference between the two checkers instead of resolving it. - AvroSchemaUtils: marked as being retired; do not add methods. Fix stale references: eight "equivalent to X" javadocs in HoodieSchemaUtils where X no longer exists, the AvroSchemaUtils.isProjectionOfInternal mention in InternalSchemaConverter, the AvroSchemaUtils.getAvroRecordQualifiedName mention in HoodieSchemaConversionUtils, and the "only used in tests" note on toJavaDate/fromJavaDate (HoodieArrayWritableSchemaUtils calls them). Cross-reference the three projection helpers, getNestedField vs findNestedField, mergeSchemas vs AvroSchemaEvolutionUtils, and the createNewSchemaField aliases of HoodieSchemaField.of. Javadoc and comments only. Part of apache#16639.
voonhous
changed the title
hudi-agent
left a comment
hudi-agent
left a comment
There was a problem hiding this comment.
Thanks for working on this! This PR is part of the #16639 schema-utility consolidation: it reroutes callers off the HoodieSchemas/HoodieSchemaUtils wrappers to the corresponding HoodieAvroUtils methods, retires AvroSchemaCache and HoodieSchemas, relocates LocalHoodieSchemaCache into common.schema (with getInstance()→create()), switches getRecordColumnValues to take HoodieSchema directly, and tightens visibility on now-internal helpers alongside javadoc cleanup. The behavioral changes trace out as behavior-preserving and all affected callers are internal or same-package tests. No correctness issues found. A few style/readability suggestions in the inline comments. Please take a look, and this should be ready for a Hudi committer or PMC member to take it from here. A couple of small readability issues: a stale caching comment after the cache was removed, and @VisibleForTesting applied inconsistently to a public method.
cc @yihua
| @@ -110,7 +109,7 @@ public class HoodieMetadataPayload implements HoodieRecordPayload<HoodieMetadata | |||
| private static final HoodieSchema HOODIE_METADATA_SCHEMA = HoodieSchemaCache.intern( | |||
| HoodieSchema.fromAvroSchema(HoodieMetadataRecord.getClassSchema())); | |||
| // Cache the Avro schema reference for O(1) equality checks during Avro.Schema -> HoodieSchema migration | |||
There was a problem hiding this comment.
🤖 nit: the comment above still says 'Cache the Avro schema reference for O(1) equality checks' but AvroSchemaCache.intern() was removed on the next line — could you update or drop it so it matches what the code actually does?
There was a problem hiding this comment.
Reworded in 557a274. The field still anchors the == fast path in getInsertValue (line 428); HoodieMetadataRecord.getClassSchema() returns the generated class's SCHEMA$ singleton, so the reference is stable without interning.
| return rewriteRecordWithNewSchema(oldRecord, newSchema, Collections.EMPTY_MAP, validate); | ||
| } | ||
|
|
||
| @VisibleForTesting |
There was a problem hiding this comment.
🤖 nit: @VisibleForTesting on a public method is a bit contradictory — the sibling gteqAvro1_10 was correctly narrowed to package-private to match the annotation's intent. Could gteqAvro1_9 follow the same treatment, or if it has real external callers, drop the annotation?
There was a problem hiding this comment.
Keeping it as is. gteqAvro1_9 has test callers outside the package and outside the module (TestHoodieLogFormat in hudi-hadoop-common, TestObjectSizeCalculator in common.util), so package-private is not an option, and the annotation is what records that the method is public only for tests -- same pattern as makeFieldNonNull and rewriteRecords in this class. gteqAvro1_10 and gteqAvro1_12 could be narrowed because their only callers are in-package or in-class.
The comment on HOODIE_METADATA_AVRO_SCHEMA still described the field as an interned cache after b00fc5b dropped AvroSchemaCache. The field remains the anchor for the == fast path in getInsertValue; HoodieMetadataRecord.getClassSchema() returns the generated class's SCHEMA$ singleton, so the reference is stable without interning. Say that instead.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #19809 +/- ##
=========================================
Coverage 78.31% 78.32%
- Complexity 33888 33904 +16
=========================================
Files 2541 2540 -1
Lines 141728 141763 +35
Branches 17182 17237 +55
=========================================
+ Hits 111001 111042 +41
- Misses 23017 23021 +4
+ Partials 7710 7700 -10
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
hudi-agent
left a comment
hudi-agent
left a comment
There was a problem hiding this comment.
Thanks for the contribution! This PR documents the routing contracts between the schema utility classes and shifts code to match them — folding HoodieSchemas into HoodieSchemaUtils, deleting the dead AvroSchemaCache, relocating LocalHoodieSchemaCache and the convertValueForSpecificDataTypes/convertBytesToBigDecimal helpers, retyping getRecordColumnValues to take HoodieSchema, and narrowing caller-less members. The substantive changes trace as behavior-preserving, with reference-equality fast paths retained and the interning removal affecting only a perf micro-optimization. No issues flagged from this automated pass — a Hudi committer or PMC member can take it from here for a final review.
cc @yihua
voonhous
left a comment
voonhous
left a comment
There was a problem hiding this comment.
Two text-only notes, no code change:
- Impact says "Visibility narrowed on 12 caller-less members"; 119e1af narrows 11 (7 private, 3 package-private, 1 constant) and adds
@VisibleForTestingto 4 that stay public. - 5be7b58's message says the eight "equivalent to X" javadocs in HoodieSchemaUtils named an X that no longer exists; four of those X's still exist (
removeFields,createNewSchemaField,createNullableSchema,asNullable). "stale or imprecise" would be accurate on the squash message.
| List<HoodieSchemaField> fields = Stream.concat( | ||
| Stream.of(createNewSchemaField( | ||
| HoodieRecord.RECORD_KEY_METADATA_FIELD, HoodieSchema.create(HoodieSchemaType.STRING), null, null)), | ||
| orderingFieldNames.stream().map(orderingFieldName -> tableSchema.getField(orderingFieldName) |
There was a problem hiding this comment.
major: pre-existing, not introduced by this move. tableSchema.getField(name) is an exact top-level lookup (HoodieSchema.java:1215), so a nested ordering field such as nested_record.level throws IllegalArgumentException here on the first delete of a v10 native-log table: nested precombine is supported (HUDI-4051) and HoodieNativeLogAppendHandle.java:93 passes the names through verbatim. Out of scope for a behavior-preserving PR; should the fix (writer createDeleteLogFieldValues and reader getValue aligned on a flat field name) go in a separate PR, with a note on #16639 meanwhile?
There was a problem hiding this comment.
Filed an issue for. this here, will fix it outside of this PR and keep this PR purely for refactoring.
#19823
hasSmallPrecisionDecimalField and isSmallPrecisionDecimalField have had no production caller since apache#13882 removed canUseRowWriter; only their own test called them. Delete both and the test, and fold hasDecimalWithCondition into hasDecimalField now that the predicate has a single value.
The HoodieSchema signature change dropped the HoodieAvroSchemaCache intern the Avro-typed version did, so the schema instance reaching BaseAvroPayload#getRecord was no longer canonical and its identity fast path could miss for callers that parse their own copy. Intern through HoodieSchemaCache, which is the same canonical instance the old path resolved to.
safeAvroToJsonString pointed at avroToJsonString, which is private now; link the public avroToJson instead. HoodieSchemaTypePromotion said it was used by the compatibility checkers while the paragraph above explains why they deliberately do not use it. AvroSchemaUtils claimed a HoodieSchema twin for every method, which does not hold for the strict getNonNullTypeFromUnion. getCachedSchema's scaladoc still named the Avro Schema type and the Avro-keyed cache.
createDeleteLogSchema: every case used one LONG field, so an implementation keeping only the first field would pass. Cover two ordering fields in caller order, no ordering fields, and an already-nullable non-null-first union, whose default is dropped by HoodieSchemaField.of. generateProjectionSchema: pin case-insensitive matching, which HiveHoodieReaderContext relies on. getRecordColumnValues: the test was a copy of the sort-column test; use the nested schema and pin a nested column and a missing one. convertBytesToBigDecimal: cover the null-schema guard. TestLsmFileGroupRecordIterator: drop the delete-log schema test, now a subset of TestHoodieSchemaUtils#testCreateDeleteLogSchema. TestLocalHoodieSchemaCache: restore the two tests deleted with the rename in apache#17740; the class had no direct test since.
github-actions
Bot
added
size:XL
voonhous
left a comment
voonhous
left a comment
There was a problem hiding this comment.
Two pre-existing bugs surfaced (hasDecimalField union recursion, default-locale lowercasing in generateProjectionSchema); both left for separate PRs to keep this one behavior-preserving, issues linked in the threads.
| return hasDecimalField(schema.getValueType()); | ||
| case UNION: | ||
| return hasDecimalWithCondition(schema.getNonNullType(), condition); | ||
| return hasDecimalField(schema.getNonNullType()); |
There was a problem hiding this comment.
major: pre-existing, not introduced by this fold: master has the same arm (HoodieSchemaUtils.java:799-800, since #17600). getNonNullType() returns this for a union without a null branch (HoodieSchema.java:1397-1399) and ["null","string","int"] reduces to exactly that shape (line 1413), so this recurses to StackOverflowError. SourceFormatAdapter.java:261 calls it on every JSON source schema; HoodieSchemaRepair.hasTimestampMillisField:249 has the identical arm. Out of scope for a behavior-preserving PR; should the getTypes().stream().anyMatch(...) fix go in a separate PR, with an issue linked here meanwhile?
There was a problem hiding this comment.
Filed as #19825 (with the HoodieSchemaRepair.hasTimestampMillisField twin); fix goes in a separate PR, this one keeps the arm as on master.
| assertTrue(fieldNames1.contains("timestamp")); | ||
|
|
||
| // Field names are matched case-insensitively; HiveHoodieReaderContext lowercases names before calling this. | ||
| HoodieSchema schema2 = HoodieSchemaUtils.generateProjectionSchema(originalSchema, Arrays.asList("_ROW_KEY")); |
There was a problem hiding this comment.
minor: not blocking, pre-existing. This pins case-insensitive matching, but generateProjectionSchema lowercases with the default locale (HoodieSchemaUtils.java:475,478) while its only production caller uses Locale.ROOT (HiveHoodieReaderContext.java:236); under tr_TR an ID column lowercases to dotless-i on one side and id on the other, and the projection throws "Field id not found in log schema". _ROW_KEY has no I, so this case cannot catch it. Should the Locale.ROOT fix and an I-bearing name (PII_COL) go in a separate PR, with an issue linked here?
There was a problem hiding this comment.
Filed as #19826; fix in a separate PR. Correction to the claim above: generateProjectionSchema has six callers, HiveHoodieReaderContext is the one that pre-lowercases (with Locale.ROOT); the others lowercase both sides through this call and stay consistent.
HoodieSchemaCache is value-keyed, so a caller whose schema instance is not the stored key pays a deep Schema.equals on every call; the sort comparator calls twice per comparison and col-stats once per record per column. Master interned through HoodieAvroSchemaCache, whose weak-key front is an identity lookup, so go back to exactly that path. Add a test that fails without the intern: a record held by a BaseAvroPayload on the interned Avro schema, read with a freshly parsed equal HoodieSchema, comes back as String only when the payload's identity fast path hits (a miss re-serializes and yields Utf8). Drop the nested column the sibling getNestedFieldVal test already pins on the same fixture.
generateProjectionSchema matched names case-insensitively without saying so; the reason lived only on the hadoop-mr twin (Hive lowercases column projections). Port it, note that the projected field keeps the schema's casing and that fields differing only in case cannot be projected. The sort-column sibling's @PARAM still named the Avro Schema type.
createDeleteLogSchema: cover a timestamp-millis and a decimal ordering field, the branch with the delete path's history (apache#13998, apache#13163, apache#12006); assert the [long, null] branch order directly instead of only through the dropped default; drop an assertion the field-name list already covers. TestHoodieAvroUtils: the moved convertValueForSpecificDataTypes tests reuse the class's DATE/TS_MILLIS/TS_MICROS schema constants. TestJsonKafkaSource: import HoodieAvroUtils instead of the fully-qualified name left over from the old target.
|
@wombatu-kun Can you please help to review this when you are free? |
| * {@code addHoodieKeyToRecord}, {@code getRecordColumnValues}, {@code createHoodieRecordFromAvro}</li> | ||
| * </ul> | ||
| * | ||
| * <p>Raw {@link Schema} helpers live here only when they serve a record operation on this class (for example |
There was a problem hiding this comment.
projectSchema is given here as a raw-Schema helper that serves a record operation on this class, but nothing on this class calls it and its only production caller is HoodieSchemaUtils.projectSchema, which the sibling javadoc lists as a delegation being retired. Could it be dropped from the example list, leaving createNewSchemaField and unwrapNullable?
There was a problem hiding this comment.
Dropped in 173cef4; the example list is now createNewSchemaField, unwrapNullable.
| /** | ||
| * Convert a given avro record to a JSON string. If the record contents are invalid, return the record.toString(). | ||
| * Use this method over {@link HoodieAvroUtils#avroToJsonString} when simply trying to print the record contents without any guarantees around their correctness. | ||
| * Use this method over {@link #avroToJson(GenericRecord, boolean)} when simply trying to print the record contents without any guarantees around their correctness. |
There was a problem hiding this comment.
avroToJson returns byte[] while safeAvroToJsonString returns String, so it is not an alternative a caller would weigh against this method - the sibling it actually wraps is the now-private avroToJsonString. Could the comparison just be dropped, since the preceding sentence already states the fallback behaviour?
| * This is also what Conversions.DecimalConversion.toBytes() outputs inside a byte buffer | ||
| */ | ||
| public static Object convertBytesToFixed(byte[] bytes, Schema schema) { | ||
| @VisibleForTesting |
There was a problem hiding this comment.
convertBytesToBigDecimal(byte[], LogicalTypes.Decimal) is in the same position as convertBytesToFixed - its only caller outside this class is TestMercifulJsonConverter, in this package - but stayed public and unannotated. rewritePrimaryType and the convertToRecord overloads are the same shape with test-only callers in other modules: could all four get the narrowing treatment this PR describes?
There was a problem hiding this comment.
Done in 0b0c95e. convertBytesToBigDecimal(byte[], Decimal) and the 4- and 6-arg convertToRecord are package-private with @VisibleForTesting (callers: this class plus TestHoodieAvroUtils / TestMercifulJsonConverter in the same package). rewritePrimaryType and the 7-arg convertToRecord keep public with @VisibleForTesting because their test callers are in other modules (TestHoodieArrayWritableSchemaUtils in hudi-hadoop-mr, TestColStatsRecordWithMetadataRecord in hudi-spark). PR body updated: 13 narrowed, and the "kept on purpose" line now names only the 5-arg createNewSchemaField.
| * | ||
| * <p>This class provides HoodieSchema equivalents of operations found in AvroSchemaUtils | ||
| * and HoodieAvroUtils, focusing on table schema management rather than record-level operations.</p> | ||
| * <p>What lives here:</p> |
There was a problem hiding this comment.
The list omits about a third of the public API - all three createNewSchemaField overloads, getFieldSchema, createHoodieWriteSchema, hasDecimalField, createNewSchemaFromFieldsWithReference and toJavaDefaultValue - and getNestedField is filed under "needs more than HoodieSchema offers on its own" although its own javadoc in this PR calls it a null-checking facade over HoodieSchema#getNestedField. Could the missing ones that fit an existing bullet be added, and getNestedField move to the "questions about a single schema" line under "Not here"?
There was a problem hiding this comment.
Reworked in 173cef4: added createHoodieWriteSchema, createNewSchemaFromFieldsWithReference, the createNewSchemaField copy factory (with the two aliases noted), toJavaDefaultValue, getRecordQualifiedName and hasDecimalField; getNestedField and getFieldSchema now sit under "questions about a single schema" as facades over the HoodieSchema instance methods. Still unlisted on purpose: addMetadataColumnTypes (test-only), createSchemaErrorString, and createNullableSchema, which the delegation paragraph already covers.
|
|
||
| @Test | ||
| public void testBasicCacheUsage() { | ||
| LocalHoodieSchemaCache schemaCache = LocalHoodieSchemaCache.create(); |
There was a problem hiding this comment.
Neither test creates two caches, so both still pass if create() returned a shared singleton - the one property the rename and its new javadoc assert. Could one case call create() twice and assert the two id spaces are independent?
There was a problem hiding this comment.
Added testCreateReturnsIndependentCaches in 7eb2cbf: two caches, the first cache's id misses in the second, and the same id resolves to a different schema in each.
convertBytesToBigDecimal(byte[], Decimal) and the 4- and 6-arg convertToRecord overloads are called only from this class and from tests in the same package, so they become package-private with @VisibleForTesting, like convertBytesToFixed. rewritePrimaryType and the 7-arg convertToRecord keep public because their test callers are in other modules (hudi-hadoop-mr, hudi-spark); the annotation records that this is the only reason.
HoodieSchemaUtils: list the public API the class javadoc skipped (createHoodieWriteSchema, createNewSchemaFromFieldsWithReference, the createNewSchemaField copy factory, toJavaDefaultValue, getRecordQualifiedName, hasDecimalField) and file getFieldSchema and getNestedField as facades over HoodieSchema instance methods instead of as lookups of their own. HoodieAvroUtils: projectSchema serves no record operation on the class, so it leaves the raw-Schema example list; the safeAvroToJsonString javadoc compared against avroToJson, which returns bytes, so the comparison goes.
The two existing cases use one cache each, so a shared singleton would still pass them. Create two caches, check the first cache's id misses in the second and that the same id resolves to a different schema in each.
getRecordColumnValues built a new Properties and an ArrayList on every call while its sibling getSortColumnValuesWithPartitionPathAndRecordKey reuses the static PROPERTIES and a fixed array. The method runs once per comparison in RDDBucketIndexPartitioner's sort comparator and once per record per column in collectColumnRangeMetadata, so the two allocations were paid N log N times per partition. Pre-existing, on the line this PR already rewrote. Also names the two callers the intern defends instead of the general "sort path" claim, and rewords the second stale "cached Avro schema reference" comment in HoodieMetadataPayload#getInsertValue.
testGetRecordColumnValues asserted null for a nested column on an empty nested record, which a broken traversal would also return; it now reads a real nested value and covers a null and a non-record intermediate. HoodieMetadataPayload#getInsertValue's reference-equality fast path had no discriminating test: the class schema singleton must return the generated HoodieMetadataRecord, while an equal but distinct schema instance rebuilds a GenericData.Record.
5 participants
Describe the issue this Pull Request addresses
Part 1 of #16639. The HoodieSchema migration moved the schema helpers out of
AvroSchemaUtils/HoodieAvroUtilsbut never wrote down what belongs where, so helpers keep being re-implemented instead of found (472bc0c48564,e3f93425cd3b, #19212). Absorbs/Closes #15908.Summary and Changelog
Behavior-preserving. One concern per commit; each compiles and reverts alone.
HoodieSchemaanswers questions about one schema and holds the factories;HoodieSchemaUtilsholds table-schema transforms and the Hudi record shapes;HoodieSchemaCompatibilityis the only compatibility/projection entry point;HoodieAvroUtilsholds Avro record and value operations;common.schema.internalis the InternalSchema domain. Look-alike helpers with different semantics cross-reference each other instead of being merged.HoodieSchemaUtils,HoodieSchemasfolds into it,AvroSchemaCacheis deleted,LocalHoodieSchemaCachemoves tocommon.schema,getRecordColumnValuestakesHoodieSchema, caller-less members are narrowed or deleted, stale "equivalent to X" javadocs are fixed.asNullable, dissolveAvroSchemaUtils, dedupe the compatibility cluster.Per-commit changelog
narrow unused schema util API: members with no caller outside their class become private/package-private; test-only public members get@VisibleForTesting. Nothing deleted, no bodies changed.move LocalHoodieSchemaCache:common.util->common.schema;getInstance()->create()(it returns a new instance per call). Sole callerRecordContext.delete AvroSchemaCache: its one caller interned a generated-class static, a no-op;HoodieAvroSchemaCacheis the live Avro-keyed cache. See the feat(schema): Phase 24 - Restore O(1) reference equality comparison i… #17672 section below.move value helpers to avro utils:HoodieSchemaUtils.convertValueForSpecificDataTypes(no production callers) deleted;convertBytesToBigDecimal(byte[], HoodieSchema)moved toHoodieAvroUtils, the(byte[], int, int)shim deleted. Tests moved, two added.column values take HoodieSchema:HoodieAvroUtils#getRecordColumnValuestookSchemaand re-wrapped it while both callers unwrapped aHoodieSchemato call it. Test added.fold HoodieSchemas into utils:createDeleteLogSchemamoves next togetRecordKeySchema; class deleted; six callers updated. Tests added.document schema util contracts: the rule above, class javadocs forHoodieSchemaRepair,HoodieAvroWrapperUtils,HoodieSchemaTypePromotion, stale references fixed, cross-references added.hasSmallPrecisionDecimalFielddeleted with its test (no production caller since refactor: Update ParquetWriteSupport for Rows to match Avro writer behavior #13882);getRecordColumnValuesinterns throughHoodieAvroSchemaCacheas before (identity-keyed front), with a test that fails without it; stale javadoc fixed and the case-insensitive projection contract documented; tests pin delete-log field order and logical types, case-insensitive projection, a missing column, the null-schema guard, andLocalHoodieSchemaCache(test restored from feat(schema): Migrate json and proto converters to use HoodieSchema #17740). Two pre-existing bugs found on the way are filed, not fixed here: [BUG] HoodieSchemaUtils.hasDecimalField recurses forever on a union with two or more non-null branches #19825, [BUG] generateProjectionSchema lowercases with the default locale while HiveHoodieReaderContext uses Locale.ROOT #19826.convertBytesToBigDecimal(byte[], Decimal)and the 4- and 6-argconvertToRecordnarrowed to package-private (callers: this class and same-package tests);rewritePrimaryTypeand the 7-argconvertToRecordmarked@VisibleForTesting(test callers in hudi-hadoop-mr and hudi-spark);HoodieSchemaUtilsclass javadoc lists the public API it skipped and filesgetFieldSchema/getNestedFieldas facades overHoodieSchema;projectSchemaand theavroToJsoncomparison dropped fromHoodieAvroUtilsjavadocs; test thatLocalHoodieSchemaCache.create()returns independent caches.getRecordColumnValuesreuses the staticPROPERTIESand a fixed array instead of allocatingProperties+ArrayListper call (pre-existing, on the line this PR rewrote; it runs per comparison inRDDBucketIndexPartitioner's sort comparator); a second stale "cached Avro schema reference" comment inHoodieMetadataPayload#getInsertValuereworded; tests pin thegetInsertValuereference-equality fast path (class schema returnsHoodieMetadataRecord, an equal-but-distinct schema returnsGenericData.Record) and a real nested value plus null / non-record intermediates ingetRecordColumnValues.Kept on purpose after re-checking callers: the 5-arg
HoodieSchemaUtils.createNewSchemaField(ExpressionPayload.mergeSchema).Why deleting AvroSchemaCache leaves the #17672 fast path unchanged
#17672 restored the
==check inHoodieMetadataPayload#getInsertValuewith two halves: pre-populatingHoodieSchemaCache(wrapper identity) and pre-populatingAvroSchemaCacheso raw Avro schemas interned byHoodieAvroDataBlockand six other call sites would resolve toHOODIE_METADATA_AVRO_SCHEMA. The migration removed every one of those consumers (#17743, #17763, #17772, #17952, last #18967), so since June the cache held one entry nothing looked up;intern(SCHEMA$)on that empty identity-loader cache returnedSCHEMA$, which is what the field now holds directly. TheHoodieSchemaCachehalf still does the work (HoodieAvroDataBlockinterns header schemas by value). Init-order hardening left for later:HOODIE_METADATA_AVRO_SCHEMA = HOODIE_METADATA_SCHEMA.toAvroSchema().Impact
Internal API only, no behavior change. Removed:
HoodieSchemas,AvroSchemaCache,HoodieSchemaUtils#convertValueForSpecificDataTypes,HoodieSchemaUtils#convertBytesToBigDecimal(byte[], int, int),HoodieSchemaUtils#hasSmallPrecisionDecimalField. Moved:LocalHoodieSchemaCache(andgetInstance()->create()),convertBytesToBigDecimal(byte[], HoodieSchema)->HoodieAvroUtils. Signature:HoodieAvroUtils#getRecordColumnValuestakesHoodieSchema. Visibility narrowed on 13 caller-less members.Risk Level
Low. Moves, visibility changes, caller-less deletions and javadoc only.
Documentation Update
none
Contributor's checklist