feat(schema): Internal Schema System Integration with HoodieSchema #14314

the-other-tim-brown · 2025-11-21T14:33:03Z

Describe the issue this Pull Request addresses

Addresses #14269
The AvroInternalSchemaConverter currently directly relies on an Avro schema. This PR switches the class to operate on HoodieSchema as part of the migration to Hoodie's own type system.

Summary and Changelog

AvroInternalSchemaConverter is renamed to InternalSchemaConverter
All methods in InternalSchemaConverter now take HoodieSchema instead of an avro schema
Callers of these methods convert their avro schema to HoodieSchema with the fromAvroSchema method
Related test cases are update to directly construct HoodieSchema directly when possible

Impact

This is laying the groundwork for moving the codebase over to Hudi's own type system.

Risk Level

Low, this maintains parity with the existing code

Documentation Update

Contributor's checklist

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

the-other-tim-brown · 2025-11-21T15:31:09Z

hudi-common/src/main/java/org/apache/hudi/common/schema/HoodieSchema.java

-   * @param type the non-null type
-   * @return new HoodieSchema representing a nullable union
-   */
-  public static HoodieSchema createNullableSchema(HoodieSchema type) {


There is another method createNullable that covers similar functionality so I am deduping

the-other-tim-brown · 2025-11-21T15:31:49Z

hudi-common/src/main/java/org/apache/hudi/common/schema/HoodieSchema.java

   */
-  public Option<String> getFullName() {
-    return Option.ofNullable(avroSchema.getFullName());
+  public String getFullName() {


The name is always meant to be non-empty in avro and I think we should follow a similar pattern here or we end up with a lot of option handling code in the callers

Can you add a precondition in the constructor of HoodieSchema to verify this assumption

This is already present: https://github.com/apache/hudi/blob/master/hudi-common/src/main/java/org/apache/hudi/common/schema/HoodieSchema.java#L231

hudi-common/src/main/java/org/apache/hudi/internal/schema/convert/InternalSchemaConverter.java

yihua · 2025-11-22T18:34:14Z

hudi-common/src/main/java/org/apache/hudi/internal/schema/convert/InternalSchemaConverter.java

+import org.apache.hudi.common.schema.HoodieJsonProperties;
+import org.apache.hudi.common.schema.HoodieSchema;
+import org.apache.hudi.common.schema.HoodieSchemaField;
+import org.apache.hudi.common.schema.HoodieSchemaType;


I start to think that it's better to remove the Hoodie prefix from these class names in org.apache.hudi.common.schema packages, so it's easier to read, as eventually we are going to only maintain one schema system. Wdyt?

Yes, it is implied by the package but I think we should do that after we have removed the avro usage. Otherwise the difference is more subtle and likely to be missed.

Sounds good.

yihua · 2025-11-22T19:10:37Z

hudi-common/src/main/java/org/apache/hudi/common/schema/HoodieSchema.java

+      return getTypes().stream()
+          .filter(schema -> schema.getType() != HoodieSchemaType.NULL)
+          .findFirst()
+          .orElseThrow(() -> new IllegalArgumentException("No non-null type found in Union"));


Use HoodieSchema#getNonNullType instead? Or is this only intended to get the type, so missing an element of a union is fine?

I'll switch to getNonNullType. I forgot to update this branch after the other one was merged.

hudi-common/src/main/java/org/apache/hudi/internal/schema/convert/InternalSchemaConverter.java

.../hudi-client-common/src/main/java/org/apache/hudi/table/action/commit/HoodieMergeHelper.java

...spark-datasource/hudi-spark-common/src/main/scala/org/apache/hudi/HoodieSparkSqlWriter.scala

.../hudi-spark/src/test/scala/org/apache/spark/sql/hudi/common/TestHoodieInternalRowUtils.scala

…ation

the-other-tim-brown · 2025-11-25T14:28:06Z

@yihua and @bvaradar is there any other feedback for this PR?

hudi-bot · 2025-11-25T20:35:03Z

CI report:

ec32e0d UNKNOWN
0a575d9 UNKNOWN
0057357 UNKNOWN
5b63f28 UNKNOWN
1aee3c5 Azure: SUCCESS

Bot commands

@hudi-bot supports the following commands:

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

yihua

LGTM

yihua · 2025-11-25T21:01:28Z

hudi-common/src/main/java/org/apache/hudi/internal/schema/convert/InternalSchemaConverter.java

+import org.apache.hudi.common.schema.HoodieJsonProperties;
+import org.apache.hudi.common.schema.HoodieSchema;
+import org.apache.hudi.common.schema.HoodieSchemaField;
+import org.apache.hudi.common.schema.HoodieSchemaType;


Sounds good.

yihua · 2025-11-25T21:03:29Z

hudi-common/src/main/java/org/apache/hudi/common/schema/HoodieSchema.java

+      return getTypes().stream()
+          .filter(schema -> schema.getType() != HoodieSchemaType.NULL)
+          .findFirst()
+          .orElseThrow(() -> new IllegalArgumentException("No non-null type found in Union"));


github-actions bot added the size:L PR with lines of changes in (300, 1000] label Nov 21, 2025

the-other-tim-brown commented Nov 21, 2025

View reviewed changes

yihua reviewed Nov 22, 2025

View reviewed changes

hudi-common/src/main/java/org/apache/hudi/internal/schema/convert/InternalSchemaConverter.java Show resolved Hide resolved

hudi-common/src/main/java/org/apache/hudi/internal/schema/convert/InternalSchemaConverter.java Show resolved Hide resolved

yihua reviewed Nov 22, 2025

View reviewed changes

yihua added this to the release-1.2.0 milestone Nov 22, 2025

the-other-tim-brown force-pushed the 14269-internal-schema-integration branch from 85f648a to 5b63f28 Compare November 23, 2025 23:20

github-actions bot added size:XL PR with lines of changes > 1000 and removed size:L PR with lines of changes in (300, 1000] labels Nov 24, 2025

the-other-tim-brown force-pushed the 14269-internal-schema-integration branch from 12cfee8 to 970cc42 Compare November 24, 2025 21:33

the-other-tim-brown added 8 commits November 25, 2025 08:01

Switch internal schema conversion to use HoodieSchema

9d4311b

fix rebase issues

d08067d

remove unrelated changes

526e9c3

don't use option for name to better match avro

0ab8c93

optimize null schema, make sure to convert to avro schema in base rel…

28c1504

…ation

fix relation factory

d236d0c

address pr feedback

e19568a

update for logical type handling

df96269

the-other-tim-brown force-pushed the 14269-internal-schema-integration branch from 2159f34 to df96269 Compare November 25, 2025 13:02

use helpers for decimal types, make sure bytes subtype is used

1aee3c5

yihua approved these changes Nov 25, 2025

View reviewed changes

yihua merged commit a06d409 into apache:master Nov 25, 2025
135 of 137 checks passed

feat(schema): Internal Schema System Integration with HoodieSchema #14314

feat(schema): Internal Schema System Integration with HoodieSchema #14314

Conversation

the-other-tim-brown commented Nov 21, 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!

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!

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!

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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

the-other-tim-brown commented Nov 25, 2025

Uh oh!

hudi-bot commented Nov 25, 2025

CI report:

Uh oh!

yihua left a comment

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

the-other-tim-brown commented Nov 21, 2025 •

edited

Loading