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


##########
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:
   This needs a test for case-insensitive schema merge of nested struct types: 
where the name of a nested field differs only in case. And the same with a type 
upcast.



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