voonhous opened a new issue, #19841: URL: https://github.com/apache/hudi/issues/19841
#16639 wrote down where schema helpers belong and removed the duplication inside the Avro/HoodieSchema util classes themselves. Two look-alike pairs are still standing outside them, and both are the failure mode that issue describes: the same logic re-implemented because the existing one was not found. ### 1. `generateProjectionSchema`, implemented twice - `HoodieSchemaUtils#generateProjectionSchema(HoodieSchema, List<String>)` -- `hudi-common/src/main/java/org/apache/hudi/common/schema/HoodieSchemaUtils.java:496` - `HoodieRealtimeRecordReaderUtils#generateProjectionSchema(HoodieSchema, Map<String, HoodieSchemaField>, List<String>)` -- `hudi-hadoop-mr/src/main/java/org/apache/hudi/hadoop/utils/HoodieRealtimeRecordReaderUtils.java:124` Same loop, same case-insensitive lookup, same exception down to the punctuation: ``` "Field " + fn + " not found in log schema. Query cannot proceed! Derived Schema Fields: " ``` The wording shows the copy direction: the general-purpose hudi-common helper still says "log schema", which only makes sense in the realtime-reader caller. Two differences a merge has to carry over: - The hadoop-mr overload takes a prebuilt `schemaFieldsMap` rather than building one. Its sole caller, `AbstractRealtimeRecordReader:186`, builds that map at `:182` via `getNameToFieldMap(writerSchema)` and also hands it to `constructHiveOrderedSchema` at `:183`, so the map is genuinely shared and worth keeping out of the helper. `getNameToFieldMap` lowercases its keys exactly as the hudi-common version does inline. - The hadoop-mr version calls the 5-arg `HoodieSchema.createRecord` and propagates `writeSchema.isError()`. The hudi-common version calls the 4-arg overload, which hard-codes `isError = false`. Suggested shape: add a `HoodieSchemaUtils#generateProjectionSchema` overload that accepts the prebuilt map and preserves `isError`, delete the hadoop-mr implementation, and point `AbstractRealtimeRecordReader` at the survivor. ### 2. The timestamp-millis check, split across two domains - `HoodieSchemaRepair#hasTimestampMillisField(HoodieSchema)` -- `hudi-common/src/main/java/org/apache/hudi/common/schema/HoodieSchemaRepair.java:232`. Recursive over a whole table schema (RECORD, ARRAY, MAP, UNION), true if any leaf is a millis timestamp. - `HoodieTableMetadataUtil#isTimestampMillisField(HoodieSchema)` -- `hudi-common/src/main/java/org/apache/hudi/metadata/HoodieTableMetadataUtil.java:506`. The same leaf test for a single field schema, after `getNonNullType()`. The leaf test is what is duplicated: unwrap the union, check `TIMESTAMP` with `TimePrecision.MILLIS`. The `HoodieSchemaRepair` class javadoc already cross-references the sibling, so the relationship is known; the walker should call the single-field predicate instead of re-implementing it, leaving one definition of "is a millis timestamp". ### Scope Both are `refactor(schema)` and behavior-preserving provided the two differences in part 1 are carried over. Existing coverage: `TestHoodieSchemaUtils`, `TestHoodieRealtimeRecordReaderUtils` and `TestHoodieRealtimeRecordReader` for part 1; `TestHoodieSchemaRepair` and `TestHoodieTableMetadataUtil` for part 2. Related: #16639 (the routing rule and the remaining Avro round-trips). -- 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]
