anew commented on code in PR #57722:
URL: https://github.com/apache/spark/pull/57722#discussion_r3713429103


##########
sql/pipelines/src/test/scala/org/apache/spark/sql/pipelines/util/SchemaInferenceUtilsSuite.scala:
##########
@@ -270,4 +270,52 @@ class SchemaInferenceUtilsSuite extends SparkFunSuite {
     assert(addedColumnNames === Set("full_name", "email"))
     assert(deletedColumnNames === Set("first_name", "last_name"))
   }
+
+  test("determineColumnChanges - case-only difference is a distinct column 
when case-sensitive") {
+    // Default (case-sensitive) behavior: `value` and `Value` are distinct 
columns. Diffing the two
+    // schemas directly therefore drops `value` and adds `Value`. (On the real 
evolution path the
+    // target is the MERGED union of current + desired, so `value` is never 
dropped; the point here
+    // is only that diffSchemas keys column identity case-sensitively by 
default -- the pre-SPARK-
+    // 58517 behavior.)
+    val currentSchema = new StructType().add("id", IntegerType).add("value", 
StringType)
+    val targetSchema = new StructType().add("id", IntegerType).add("Value", 
StringType)
+
+    val changes =
+      SchemaInferenceUtils.diffSchemas(currentSchema, targetSchema, 
caseSensitive = true)
+
+    val addChanges = changes.collect { case ac: TableChange.AddColumn => 
ac.fieldNames()(0) }
+    val deleteChanges = changes.collect { case dc: TableChange.DeleteColumn => 
dc.fieldNames()(0) }
+    assert(addChanges === Seq("Value"))
+    assert(deleteChanges === Seq("value"))
+  }
+
+  test("determineColumnChanges - case-only difference is a no-op when 
case-insensitive") {
+    // With caseSensitive = false, `Value` matches the existing `value`: same 
type, nullability, and
+    // comment, so there is no change at all -- crucially NOT a drop-then-add, 
and no rename.
+    val currentSchema = new StructType().add("id", IntegerType).add("value", 
StringType)
+    val targetSchema = new StructType().add("id", IntegerType).add("Value", 
StringType)
+
+    val changes =
+      SchemaInferenceUtils.diffSchemas(currentSchema, targetSchema, 
caseSensitive = false)
+
+    assert(changes.isEmpty, s"expected no changes, got: $changes")
+  }
+
+  test("determineColumnChanges - case-insensitive match addresses the current 
column name for a " +
+    "type change") {
+    // The target field differs from the current one only in case AND changes 
type. Under
+    // case-insensitive matching this is a single in-place update, and the 
change must address the
+    // column by its CURRENT (already-persisted) name `value`, not the 
target's `Value`.
+    val currentSchema = new StructType().add("value", IntegerType)
+    val targetSchema = new StructType().add("Value", LongType)
+
+    val changes =
+      SchemaInferenceUtils.diffSchemas(currentSchema, targetSchema, 
caseSensitive = false)
+
+    assert(changes.length === 1)
+    val typeChange = changes.collect { case tc: TableChange.UpdateColumnType 
=> tc }
+    assert(typeChange.length === 1)
+    assert(typeChange.head.fieldNames() === Array("value"))
+    assert(typeChange.head.newDataType() === LongType)
+  }

Review Comment:
   I stand corrected: upcast is not supported, but `diffSchema` should report a 
type change for the nested field, rather than a field name mismatch. 



-- 
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]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to