feat(schema): Migrate HoodieFileGroupReader and related classes to use HoodieSchema #17536

the-other-tim-brown · 2025-12-09T19:35:57Z

Describe the issue this Pull Request addresses

Addresses #17469
The HoodieFileGroupReader is our main reader abstraction but it is still using the Avro schema instead of the internal HoodieSchema.

Summary and Changelog

Updates the HoodieFileGroupReader and FileGroupReaderSchemaHandler to operate solely on HoodieSchema instead of the Avro schema class.
Updates callers to pass in HoodieSchema. If the caller requires some schema manipulation or fetching before the call to the HoodieFileGroupReader, this is also updated to use HoodieSchema

Impact

Migrates core reader paths to use new HoodieSchema

Risk Level

Low

Documentation Update

Contributor's checklist

Read through contributor's guide
Enough context is provided in the sections above
Adequate tests were added if applicable

voonhous

Added some minor NIT comments.

...t/hudi-flink-client/src/main/java/org/apache/hudi/util/OrderingValueEngineTypeConverter.java

...ient/hudi-java-client/src/test/java/org/apache/hudi/client/TestJavaHoodieBackedMetadata.java

the-other-tim-brown · 2025-12-10T18:31:16Z

@hudi-bot run azure

hudi-bot · 2025-12-11T04:21:33Z

CI report:

31461f2 Azure: SUCCESS

Bot commands

@hudi-bot supports the following commands:

@hudi-bot run azure re-run the last Azure build

voonhous · 2025-12-11T14:06:01Z

LGTM. @balaji-varadarajan-ai @yihua do you wanna take another pass?

yihua · 2025-12-11T23:17:57Z

...di-client-common/src/main/java/org/apache/hudi/metadata/HoodieBackedTableMetadataWriter.java

+      HoodieSchema dataSchema = HoodieSchemaCache.intern(HoodieSchemaUtils.addMetadataFields(HoodieSchema.parse(dataWriteConfig.getWriteSchema()), dataWriteConfig.allowOperationMetadataField()));
+      HoodieSchema requestedSchema = metaClient.getTableConfig().populateMetaFields() ? getRecordKeySchema()
+          : HoodieSchemaUtils.projectSchema(dataSchema, Arrays.asList(metaClient.getTableConfig().getRecordKeyFields().orElse(new String[0])));


To follow up in a separate PR: We should start adding more methods in HoodieSchema so we can avoid nested method calls (i.e., AUtil.method1(BUtil.method2(C.method3(x), y), z) which can be improved by x.method(y, z)) that reduce readability.

The idea was to have this be an easy replacement by copying over the methods to similarly named classes. I agree we can avoid these utils in the future though.

...lient-common/src/main/java/org/apache/hudi/metadata/SecondaryIndexRecordGenerationUtils.java

...n/java/org/apache/hudi/client/clustering/run/strategy/MultipleSparkJobExecutionStrategy.java

yihua · 2025-12-12T01:06:43Z

hudi-common/src/main/java/org/apache/hudi/common/model/HoodieRecordMerger.java

 import org.apache.hudi.common.table.read.BufferedRecord;
 import org.apache.hudi.common.util.Option;

-import org.apache.avro.Schema;


Curious to see how many classes still import org.apache.avro.Schema after quite a few refactoring PRs

We probably have at least 10 more PRs

Quite a lot of refactoring, reminding me of the changes introducing the HoodieStorage abstraction :)

hudi-common/src/main/java/org/apache/hudi/common/table/PartitionPathParser.java

yihua · 2025-12-12T01:20:24Z

hudi-common/src/main/java/org/apache/hudi/common/table/log/BaseHoodieLogRecordReader.java

      // Iterate over the paths
      logFormatReaderWrapper = new HoodieLogFormatReader(storage, logFiles,
-          readerSchema, reverseReader, bufferSize, shouldLookupRecords(), recordKeyField, internalSchema);
+          readerSchema.toAvroSchema(), reverseReader, bufferSize, shouldLookupRecords(), recordKeyField, internalSchema);


Should log reader also take HoodieSchema? Is the related refactoring separated to another PR?

Yes, I started a separate branch to keep the size of this change smaller: #17548

yihua · 2025-12-12T06:25:14Z

...rg/apache/spark/sql/execution/datasources/parquet/HoodieFileGroupReaderBasedFileFormat.scala

+                .withDataSchema(HoodieSchema.fromAvroSchema(dataAvroSchema))
+                .withRequestedSchema(HoodieSchema.fromAvroSchema(requestedAvroSchema))


Could the Avro schema conversion be removed in a separate PR so the StructType schema is directly converted to HoodieSchema?

val requestedSchema = StructType(requiredSchema.fields ++ partitionSchema.fields.filter(f => mandatoryFields.contains(f.name))) val requestedAvroSchema = AvroSchemaUtils.pruneDataSchema(avroTableSchema, AvroConversionUtils.convertStructTypeToAvroSchema(requestedSchema, sanitizedTableName), exclusionFields) val dataAvroSchema = AvroSchemaUtils.pruneDataSchema(avroTableSchema, AvroConversionUtils.convertStructTypeToAvroSchema(dataSchema, sanitizedTableName), exclusionFields)

@rahil-c can you add this to your PR for the spark reader changes?

yihua

LGTM

github-actions bot added the size:XL PR with lines of changes > 1000 label Dec 9, 2025

the-other-tim-brown force-pushed the hoodie-schema-file-group-reader branch from 9a2446d to 467eac4 Compare December 10, 2025 02:49

voonhous reviewed Dec 10, 2025

View reviewed changes

...t/hudi-flink-client/src/main/java/org/apache/hudi/util/OrderingValueEngineTypeConverter.java Show resolved Hide resolved

...ient/hudi-java-client/src/test/java/org/apache/hudi/client/TestJavaHoodieBackedMetadata.java Show resolved Hide resolved

voonhous mentioned this pull request Dec 10, 2025

feat(schema): phase 17 - Remove AvroSchemaUtils usage (part 1) #17535

Merged

3 tasks

apache deleted a comment from hudi-bot Dec 10, 2025

the-other-tim-brown added 7 commits December 10, 2025 21:30

migrate file group reader and related classes to use HoodieSchema

8f130bd

add test, minor style

4050fc6

fix index manager

482bea2

fix test check, fix field lookup

1d28cd4

fix test issues

20462e4

fix getField == null checks

617baf5

fix npe on log reader

31461f2

the-other-tim-brown force-pushed the hoodie-schema-file-group-reader branch from 7ba5787 to 31461f2 Compare December 11, 2025 02:30

voonhous approved these changes Dec 11, 2025

View reviewed changes

yihua reviewed Dec 11, 2025

View reviewed changes

...n/java/org/apache/hudi/client/clustering/run/strategy/MultipleSparkJobExecutionStrategy.java Show resolved Hide resolved

yihua reviewed Dec 12, 2025

View reviewed changes

yihua approved these changes Dec 12, 2025

View reviewed changes

yihua merged commit 1479548 into apache:master Dec 12, 2025
130 of 137 checks passed

		.withDataSchema(HoodieSchema.fromAvroSchema(dataAvroSchema))
		.withRequestedSchema(HoodieSchema.fromAvroSchema(requestedAvroSchema))

feat(schema): Migrate HoodieFileGroupReader and related classes to use HoodieSchema #17536

feat(schema): Migrate HoodieFileGroupReader and related classes to use HoodieSchema #17536

Uh oh!

Conversation

the-other-tim-brown commented Dec 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Describe the issue this Pull Request addresses

Summary and Changelog

Impact

Risk Level

Documentation Update

Contributor's checklist

Uh oh!

voonhous left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

the-other-tim-brown commented Dec 10, 2025

Uh oh!

hudi-bot commented Dec 11, 2025

CI report:

Uh oh!

voonhous commented Dec 11, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yihua left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

the-other-tim-brown commented Dec 9, 2025 •

edited

Loading