You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "anew (via GitHub)" <gi...@apache.org> on 2026/08/04 01:07:44 UTC

[PR] [SPARK-58525][SQL] Propagate caseSensitive into nested field merges in n StructType.merge [spark]

anew opened a new pull request, #57733:
URL: https://github.com/apache/spark/pull/57733

   ### What changes were proposed in this pull request?
     
     `StructType.merge(left, right, caseSensitive)` honors the `caseSensitive` flag only for top-level struct fields. When it recurses into a  matched field's type, it calls `merge(leftType, rightType)` without passing `caseSensitive`, so the parameter falls back to its `= true` default:
     
     ```scala
     leftField.copy(
       dataType = merge(leftType, rightType),   // caseSensitive defaults back to true
       nullable = leftNullable || rightNullable)
   ```
     
     As a result, a field whose name differs only in case, nested inside a matched `struct` / `array<struct>` / `map<_, struct>`, is treated
     as a distinct field even under case-insensitive merging -- producing a merged schema that carries both spellings (e.g. `value` and
     `Value`).
     
     This PR forwards `caseSensitive` through the recursive call (`merge(leftType, rightType, caseSensitive)`) so nested fields fold
     case-only differences onto the existing (left) field, consistent with the top-level behavior.
     
     The `caseSensitive` parameter was originally added by SPARK-45346 ("Parquet schema inference should respect case sensitive flag when
     merging schema"), which fixed the top-level case but left the recursive call unchanged; the recursive merge itself predates any
     case-sensitivity concept (SPARK-5182, 2015). This change completes SPARK-45346's intent for nested types. 
     
     ### Why are the changes needed?
     
     It is a correctness bug affecting every caller of the case-insensitive `StructType.merge` overload, not just one subsystem. The most  direct impact is parquet/orc schema merging: `SchemaMergeUtils` passes the session's `caseSensitive` to `StructType.merge`, but nested  case-only-differing fields were still merged case-sensitively, so a later case-insensitive duplicate-column check could reject files
     that should merge. This is the same class of bug SPARK-45346 fixed at the top level, still latent for nested types.
     
     ### Does this PR introduce _any_ user-facing change?
     
     Yes, a bug fix. Under case-insensitive resolution, merging two schemas whose nested (struct/array/map) fields differ only in case now  yields a single field (keeping the left/existing spelling) instead of two conflicting fields. This makes nested schema merging  consistent with top-level merging and with the engine's case-insensitive resolver. Case-sensitive merging (the default) is unchanged.
     
     ### How was this patch tested?
     
     Added a `StructTypeSuite` test covering top-level and nested case-insensitive merges -- struct, `array<struct>`, and `map<_, struct>` -- asserting the case-only field folds onto the existing one, plus a case-sensitive control that keeps both spellings. Ran   `StructTypeSuite` and `DataTypeSuite` (catalyst), and `ParquetSchemaSuite` / `OrcSourceSuite` (the parquet/orc callers of the  case-insensitive overload) to confirm no regressions.
     
     ### Was this patch authored or co-authored using generative AI tooling?
     
     Generated-by: Claude Opus 4.8
   


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-58525][SQL] Propagate caseSensitive into nested field merges in n StructType.merge [spark]

Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #57733:
URL: https://github.com/apache/spark/pull/57733#discussion_r3709899159


##########
sql/catalyst/src/test/scala/org/apache/spark/sql/types/StructTypeSuite.scala:
##########
@@ -506,6 +506,45 @@ class StructTypeSuite extends SparkFunSuite with SQLHelper {
     assert(struct.toString() === "StructType(StructField(a,IntegerType,true))")
   }
 
+  test("SPARK-58525: case-insensitive merge folds case-only field names, including nested") {
+    // Top-level: a case-only difference merges into a single field keeping the left name.
+    assert(
+      new StructType().add("value", StringType)
+        .merge(new StructType().add("Value", StringType), caseSensitive = false) ===
+        new StructType().add("value", StringType))
+
+    // The case-sensitive default keeps both as distinct fields.
+    assert(
+      new StructType().add("value", StringType)
+        .merge(new StructType().add("Value", StringType)) ===
+        new StructType().add("value", StringType).add("Value", StringType))
+
+    // Nested struct: the flag must propagate through the recursive merge (SPARK-58525), so a
+    // case-only difference nested under a matched parent also folds rather than producing both.
+    assert(
+      new StructType().add("s", new StructType().add("value", StringType))
+        .merge(
+          new StructType().add("s", new StructType().add("Value", StringType)),
+          caseSensitive = false) ===
+        new StructType().add("s", new StructType().add("value", StringType)))
+
+    // array<struct> element recursion.
+    assert(
+      new StructType().add("a", ArrayType(new StructType().add("value", StringType)))
+        .merge(
+          new StructType().add("a", ArrayType(new StructType().add("Value", StringType))),
+          caseSensitive = false) ===
+        new StructType().add("a", ArrayType(new StructType().add("value", StringType))))
+
+    // map<_, struct> value recursion.

Review Comment:
   Resolved by the added `map<struct, _>` key-path assertion. Thanks for adding it.



-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-58525][SQL] Propagate caseSensitive into nested field merges in n StructType.merge [spark]

Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on PR #57733:
URL: https://github.com/apache/spark/pull/57733#issuecomment-5175297341

   **Merge Summary:**
   - merged into master https://github.com/apache/spark/commit/16a057cffa37ac67e61486241b75b23b9977f2d5
   - merged into branch-4.x https://github.com/apache/spark/commit/a5b0817989aafa6276052d848c5bb9ae07d4f679
   
   *Posted by `merge_spark_pr.py`*


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-58525][SQL] Propagate caseSensitive into nested field merges in n StructType.merge [spark]

Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #57733:
URL: https://github.com/apache/spark/pull/57733#discussion_r3708819741


##########
sql/catalyst/src/test/scala/org/apache/spark/sql/types/StructTypeSuite.scala:
##########
@@ -506,6 +506,45 @@ class StructTypeSuite extends SparkFunSuite with SQLHelper {
     assert(struct.toString() === "StructType(StructField(a,IntegerType,true))")
   }
 
+  test("SPARK-58525: case-insensitive merge folds case-only field names, including nested") {
+    // Top-level: a case-only difference merges into a single field keeping the left name.
+    assert(
+      new StructType().add("value", StringType)
+        .merge(new StructType().add("Value", StringType), caseSensitive = false) ===
+        new StructType().add("value", StringType))
+
+    // The case-sensitive default keeps both as distinct fields.
+    assert(
+      new StructType().add("value", StringType)
+        .merge(new StructType().add("Value", StringType)) ===
+        new StructType().add("value", StringType).add("Value", StringType))
+
+    // Nested struct: the flag must propagate through the recursive merge (SPARK-58525), so a
+    // case-only difference nested under a matched parent also folds rather than producing both.
+    assert(
+      new StructType().add("s", new StructType().add("value", StringType))
+        .merge(
+          new StructType().add("s", new StructType().add("Value", StringType)),
+          caseSensitive = false) ===
+        new StructType().add("s", new StructType().add("value", StringType)))
+
+    // array<struct> element recursion.
+    assert(
+      new StructType().add("a", ArrayType(new StructType().add("value", StringType)))
+        .merge(
+          new StructType().add("a", ArrayType(new StructType().add("Value", StringType))),
+          caseSensitive = false) ===
+        new StructType().add("a", ArrayType(new StructType().add("value", StringType))))
+
+    // map<_, struct> value recursion.

Review Comment:
   Please add a `map<struct, _>` key case too. `mergeInternal` recurses through keys and values independently, and struct keys are valid, so this value-only case will not catch a regression in the key branch.



-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-58525][SQL] Propagate caseSensitive into nested field merges in n StructType.merge [spark]

Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on PR #57733:
URL: https://github.com/apache/spark/pull/57733#issuecomment-5175307382

   also backported to 4.3


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-58525][SQL] Propagate caseSensitive into nested field merges in n StructType.merge [spark]

Posted by "anew (via GitHub)" <gi...@apache.org>.
anew commented on code in PR #57733:
URL: https://github.com/apache/spark/pull/57733#discussion_r3709503864


##########
sql/catalyst/src/test/scala/org/apache/spark/sql/types/StructTypeSuite.scala:
##########
@@ -506,6 +506,45 @@ class StructTypeSuite extends SparkFunSuite with SQLHelper {
     assert(struct.toString() === "StructType(StructField(a,IntegerType,true))")
   }
 
+  test("SPARK-58525: case-insensitive merge folds case-only field names, including nested") {
+    // Top-level: a case-only difference merges into a single field keeping the left name.
+    assert(
+      new StructType().add("value", StringType)
+        .merge(new StructType().add("Value", StringType), caseSensitive = false) ===
+        new StructType().add("value", StringType))
+
+    // The case-sensitive default keeps both as distinct fields.
+    assert(
+      new StructType().add("value", StringType)
+        .merge(new StructType().add("Value", StringType)) ===
+        new StructType().add("value", StringType).add("Value", StringType))
+
+    // Nested struct: the flag must propagate through the recursive merge (SPARK-58525), so a
+    // case-only difference nested under a matched parent also folds rather than producing both.
+    assert(
+      new StructType().add("s", new StructType().add("value", StringType))
+        .merge(
+          new StructType().add("s", new StructType().add("Value", StringType)),
+          caseSensitive = false) ===
+        new StructType().add("s", new StructType().add("value", StringType)))
+
+    // array<struct> element recursion.
+    assert(
+      new StructType().add("a", ArrayType(new StructType().add("value", StringType)))
+        .merge(
+          new StructType().add("a", ArrayType(new StructType().add("Value", StringType))),
+          caseSensitive = false) ===
+        new StructType().add("a", ArrayType(new StructType().add("value", StringType))))
+
+    // map<_, struct> value recursion.

Review Comment:
     Fix: added a `map<struct, _>` key-path case — a struct key with `value` vs `Value` under `caseSensitive = false` folds to a single `value` field,  mirroring the value-path assertion. Verified the test passes.



-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-58525][SQL] Propagate caseSensitive into nested field merges in n StructType.merge [spark]

Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan closed pull request #57733: [SPARK-58525][SQL] Propagate caseSensitive into nested field merges in n StructType.merge
URL: https://github.com/apache/spark/pull/57733


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org