danny0405 commented on code in PR #20492:
URL: https://github.com/apache/hudi/pull/20492#discussion_r4442530282
##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/spark/sql/hudi/catalog/HoodieCatalog.scala:
##########
@@ -229,7 +230,20 @@ class HoodieCatalog extends DelegatingCatalogExtension
case _ => false
}
val catalogPropertyChanges = if (propertyChanges.isEmpty) Seq.empty
else catalogPropertyChangesToApply(table, propertyChanges)
- otherChanges.groupBy(c => c.getClass).foreach {
+ // SDP reconciles its business schema against the catalog's schema on
each refresh.
+ // Hudi owns its metadata columns; deleting them must not change the
stored schema.
+ val schemaChanges = otherChanges.filterNot {
+ case delete: DeleteColumn =>
+ delete.fieldNames().length == 1 &&
HoodieRecord.HOODIE_META_COLUMNS.contains(delete.fieldNames()(0))
Review Comment:
Addressed in 7cee52b7201: switched to `HOODIE_META_COLUMNS_WITH_OPERATION`.
The regression now writes tables with operation metadata both enabled and
disabled, uses the V2 schema path, explicitly asserts whether
`_hoodie_operation` is present, and repeats metadata deletion reconciliation
three times while preserving the schema. All 11 TestHoodieCatalogDDL tests pass
on Spark 4.1 / Scala 2.13 / JDK 17.
##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/spark/sql/hudi/catalog/HoodieCatalog.scala:
##########
@@ -229,7 +230,20 @@ class HoodieCatalog extends DelegatingCatalogExtension
case _ => false
}
val catalogPropertyChanges = if (propertyChanges.isEmpty) Seq.empty
else catalogPropertyChangesToApply(table, propertyChanges)
- otherChanges.groupBy(c => c.getClass).foreach {
+ // SDP reconciles its business schema against the catalog's schema on
each refresh.
+ // Hudi owns its metadata columns; deleting them must not change the
stored schema.
+ val schemaChanges = otherChanges.filterNot {
+ case delete: DeleteColumn =>
+ delete.fieldNames().length == 1 &&
HoodieRecord.HOODIE_META_COLUMNS.contains(delete.fieldNames()(0))
+ case _ => false
+ }
+ // Reject unsupported changes before applying any supported change in
the same request.
+ schemaChanges.foreach {
+ case _: AddColumn | _: UpdateColumnType | _: UpdateColumnComment =>
Review Comment:
Addressed in 7cee52b7201. Real type changes are now rejected during
pre-validation, before the grouped schema changes or property updates run.
Added mixed add/widen regressions in both input orders and verified the schema
stays unchanged after rejection. The catalog suite passes all 11 tests.
##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/spark/sql/hudi/catalog/HoodieCatalog.scala:
##########
@@ -229,7 +230,20 @@ class HoodieCatalog extends DelegatingCatalogExtension
case _ => false
}
val catalogPropertyChanges = if (propertyChanges.isEmpty) Seq.empty
else catalogPropertyChangesToApply(table, propertyChanges)
- otherChanges.groupBy(c => c.getClass).foreach {
+ // SDP reconciles its business schema against the catalog's schema on
each refresh.
+ // Hudi owns its metadata columns; deleting them must not change the
stored schema.
+ val schemaChanges = otherChanges.filterNot {
+ case delete: DeleteColumn =>
+ delete.fieldNames().length == 1 &&
HoodieRecord.HOODIE_META_COLUMNS.contains(delete.fieldNames()(0))
+ case _ => false
+ }
+ // Reject unsupported changes before applying any supported change in
the same request.
+ schemaChanges.foreach {
+ case _: AddColumn | _: UpdateColumnType | _: UpdateColumnComment =>
+ case change =>
Review Comment:
Agreed; addressed in 7cee52b7201. Nullability requests for existing fields
are now a no-op in either direction, preserving the stored schema without
claiming NOT NULL enforcement. The regression starts with a non-nullable column
and repeats false/true nullability reconciliation through the V2 catalog while
checking schema preservation. Missing-field requests remain rejected. All 11
catalog tests pass; end-to-end MV refresh acceptance remains tracked separately
in #20405.
--
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]
