danny0405 commented on code in PR #19665:
URL: https://github.com/apache/hudi/pull/19665#discussion_r3826906898
##########
hudi-spark-datasource/hudi-spark/src/test/scala/org/apache/hudi/functional/TestCOWDataSource.scala:
##########
@@ -1979,35 +1980,39 @@ class TestCOWDataSource extends
HoodieSparkClientTestBase with ScalaAssertionSup
HoodieWriteConfig.KEYGENERATOR_CLASS_NAME.key() ->
"org.apache.hudi.keygen.ComplexKeyGenerator",
KeyGeneratorOptions.HIVE_STYLE_PARTITIONING_ENABLE.key() -> "true",
HiveSyncConfigHolder.HIVE_SYNC_ENABLED.key() -> "false",
- HoodieWriteConfig.RECORD_MERGE_IMPL_CLASSES.key() ->
"org.apache.hudi.DefaultSparkRecordMerger"
+ HoodieWriteConfig.RECORD_MERGE_IMPL_CLASSES.key() ->
"org.apache.hudi.DefaultSparkRecordMerger",
+ HoodieCommonConfig.RECONCILE_SCHEMA.key() -> "true",
Review Comment:
Addressed in . The functional test now parameterizes both and
schema-on-read and covers all four combinations. is not required: all four
cases pass, confirming that new-field nullability is applied during
canonicalization before either reconciliation path is selected.
##########
hudi-common/src/main/java/org/apache/hudi/common/schema/internal/utils/AvroSchemaEvolutionUtils.java:
##########
@@ -330,9 +332,28 @@ public static HoodieSchema
reconcileSchemaRequirements(HoodieSchema sourceSchema
List<String> nullableUpdateColsInSource = new ArrayList<>();
List<String> typeUpdateColsInSource = new ArrayList<>();
+
+ // Only relax the topmost field in a wholly new subtree. Relaxing every
descendant would alter the
+ // element/field constraints supplied by the writer instead of only making
the evolved field backfillable.
+ Set<String> visitedNewColumns = new HashSet<>();
+ colNamesSourceSchema.stream()
+ .filter(field -> !colNamesTargetSchema.contains(field))
+ .filter(field -> !META_FIELD_NAMES.contains(field))
+ .sorted()
+ .forEach(field -> {
+ String parent = TableChangesHelper.getParentName(field);
+ if (!visitedNewColumns.contains(parent)) {
+ nullableUpdateColsInSource.add(field);
+ }
+ visitedNewColumns.add(field);
+ });
+
colNamesSourceSchema.forEach(field -> {
- // handle columns that needs to be made nullable
- if (colNamesTargetSchema.contains(field) &&
sourceInternalSchema.findField(field).isOptional() !=
targetInternalSchema.findField(field).isOptional()) {
+ // Reconcile nullability only for existing user columns. Metadata fields
may be present in the source even
+ // though the target schema used for canonicalization has intentionally
stripped them.
+ if (colNamesTargetSchema.contains(field)
+ && !META_FIELD_NAMES.contains(field)
Review Comment:
Addressed in . I now build once by filtering , then use that list for both
new-field detection and existing-field nullability/type reconciliation. This
removes the redundant per-condition metadata guard and keeps metadata fields
out of all processing in this method.
##########
hudi-common/src/main/java/org/apache/hudi/common/schema/internal/utils/AvroSchemaEvolutionUtils.java:
##########
@@ -302,17 +302,19 @@ public static SchemaCompatibilityException
timestampPrecisionChangeError(String
* {@code target} one. Source is considered to be new incoming schema, while
target could refer to prev table schema.
* For example,
* if colA in source is non-nullable, but is nullable in target, output
schema will have colA as nullable.
- * if "hoodie.datasource.write.new.columns.nullable" is set to true and if
colB is not present in source, but
- * is present in target, output schema will have colB as nullable.
+ * if colB is present in source, but not in target, output schema will have
colB as nullable. If colB is a complex
+ * type, its existing descendants retain their nullability constraints.
Review Comment:
Addressed in the PR description. The Impact and Documentation Update
sections now explicitly mark a release note as required and describe the
default flip: newly added required fields previously failed with and now
succeed by being widened to nullable with a default.
--
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]
