voonhous commented on code in PR #19810:
URL: https://github.com/apache/hudi/pull/19810#discussion_r3923978571
##########
hudi-common/src/main/java/org/apache/hudi/common/schema/HoodieSchemaUtils.java:
##########
@@ -248,36 +251,42 @@ public static HoodieSchema mergeSchemas(HoodieSchema
sourceSchema, HoodieSchema
}
/**
- * Creates a nullable version of the given schema (union of null and the
schema).
+ * Create a new schema by force changing all the top-level fields as
nullable.
*
- * <p>{@link HoodieSchema#createNullable(HoodieSchema)} is the idempotent
native equivalent and is
- * preferred; this overload round-trips through Avro and is retained only
for existing call sites.</p>
+ * <p>The rewrite runs through the field-id {@link InternalSchema}: the
record is converted, every
+ * still-required top-level field is marked nullable with a {@link
TableChanges.ColumnUpdateChange},
+ * and the updated InternalSchema is converted back under the original full
name. Only the top level
+ * changes - the inner fields of a nested record keep the nullability they
had. Because the record is
+ * rebuilt from the InternalSchema, its full name, field order and per-field
docs survive, while the
+ * record-level doc and any custom record properties do not.</p>
*
- * @param schema the input schema
- * @return new HoodieSchema that allows null values
- * @throws IllegalArgumentException if schema is null
- */
- public static HoodieSchema createNullableSchema(HoodieSchema schema) {
- ValidationUtils.checkArgument(schema != null, "Schema cannot be null");
-
- // Delegate to AvroSchemaUtils
- Schema nullableAvro =
AvroSchemaUtils.createNullableSchema(schema.toAvroSchema());
- return HoodieSchema.fromAvroSchema(nullableAvro);
- }
-
- /**
- * Create a new schema by force changing all the fields as nullable.
+ * <p>When every top-level field is already nullable the input instance
itself is returned and no
+ * conversion runs.</p>
*
- * @return a new schema with all the fields updated as nullable
+ * @param schema original schema
+ * @return a schema with all the top-level fields updated as nullable, or
{@code schema} itself when
+ * there is nothing to change
* @throws IllegalArgumentException if schema is null
- * @see AvroSchemaUtils#asNullable(Schema)
*/
public static HoodieSchema asNullable(HoodieSchema schema) {
ValidationUtils.checkArgument(schema != null, "Schema cannot be null");
- // Delegate to AvroSchemaUtils
- Schema nullableAvro = AvroSchemaUtils.asNullable(schema.toAvroSchema());
- return HoodieSchema.fromAvroSchema(nullableAvro);
+ // NOTE: HoodieSchema#isNullable is false for a bare NULL type, unlike
Avro's Schema#isNullable, so a
+ // NULL-typed field is excluded explicitly to keep it out of the
update list as it always was.
+ List<String> requiredCols = schema.getFields().stream()
+ .filter(f -> !(f.schema().isNullable() || f.schema().getType() ==
HoodieSchemaType.NULL))
+ .map(HoodieSchemaField::name)
+ .collect(Collectors.toList());
+ if (requiredCols.isEmpty()) {
+ return schema;
+ }
+
+ InternalSchema internalSchema = InternalSchemaConverter.convert(schema);
Review Comment:
Kept as on master: the conversion is unchanged here and #19833 tracks the
fix (sentinel ids for the reference fields plus tolerance for the ids already
persisted by schema-on-read tables).
--
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]
