(spark) branch branch-4.x updated: [SPARK-58640][SQL] Align getBool with the sibling option accessors in CSVOptions and XmlOptions

[ur...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

uros-b pushed a commit to branch branch-4.x
in repository https://gitbox.apache.org/repos/asf/spark.git


The following commit(s) were added to refs/heads/branch-4.x by this push:
     new 425614387b3e [SPARK-58640][SQL] Align getBool with the sibling option accessors in CSVOptions and XmlOptions
425614387b3e is described below

commit 425614387b3e1df2a03fcbce5a5aecec7c8a8c6c
Author: Uros Bojanic <22...@users.noreply.github.com>
AuthorDate: Fri Aug 7 16:10:01 2026 +0000

    [SPARK-58640][SQL] Align getBool with the sibling option accessors in CSVOptions and XmlOptions
    
    ### What changes were proposed in this pull request?
    
    Rewrites the private `getBool` in `CSVOptions` and `XmlOptions` to match the shape of the sibling option accessors:
    
    ```scala
      private def getBool(paramName: String, default: Boolean = false): Boolean = {
        val paramValue = parameters.get(paramName)
        paramValue match {
          case None => default
          case Some(null) => default
          case Some(value) => value.toLowerCase(Locale.ROOT) match {
            case "true" => true
            case "false" => false
            case _ => throw QueryExecutionErrors.paramIsNotBooleanValueError(paramName)
          }
        }
      }
    ```
    
    ### Why are the changes needed?
    
    `getChar` and `getInt` in `CSVOptions` already match on `parameters.get(paramName)` and handle the missing and null cases as `case None` and `case Some(null)`, while `getBool` was an if/else-if chain over `parameters.getOrElse(paramName, default.toString)`. Besides being inconsistent, that form converts the default to a string only to parse it straight back, and calls `toLowerCase` twice on the `false` and error paths.
    
    `XmlOptions` carried a byte-identical copy of the old method, so both are updated together to keep them in sync rather than leaving them divergent.
    
    ### Does this PR introduce _any_ user-facing change?
    
    No. Behavior is unchanged for every input: an absent key yields the default, an explicit `null` value yields the default, `true`/`false` are accepted case-insensitively via `Locale.ROOT`, and any other value throws the same error with the same parameter as before.
    
    ### How was this patch tested?
    
    Behavior-preserving change covered by the existing CSV and XML option tests. Equivalence with the previous implementation was checked case by case across absent keys, explicit nulls, mixed-case `true`/`false`, the empty string, and other invalid values, for both `default` values.
    
    ### Was this patch authored or co-authored using generative AI tooling?
    
    Generated-by: Claude Code (Opus 4.8)
    
    Closes #57847 from uros-b/refactor-csvoptions-getbool.
    
    Authored-by: Uros Bojanic <22...@users.noreply.github.com>
    Signed-off-by: Uros Bojanic <22...@users.noreply.github.com>
    (cherry picked from commit 551a78334de9fe2d5cd6aa6a3dc5d4596c79dadb)
    Signed-off-by: Uros Bojanic <22...@users.noreply.github.com>
---
 .../org/apache/spark/sql/catalyst/csv/CSVOptions.scala | 18 +++++++++---------
 .../org/apache/spark/sql/catalyst/xml/XmlOptions.scala | 18 +++++++++---------
 2 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/csv/CSVOptions.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/csv/CSVOptions.scala
index 30b6877d3a82..7db03a8a2323 100644
--- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/csv/CSVOptions.scala
+++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/csv/CSVOptions.scala
@@ -107,15 +107,15 @@ class CSVOptions(
   }
 
   private def getBool(paramName: String, default: Boolean = false): Boolean = {
-    val param = parameters.getOrElse(paramName, default.toString)
-    if (param == null) {
-      default
-    } else if (param.toLowerCase(Locale.ROOT) == "true") {
-      true
-    } else if (param.toLowerCase(Locale.ROOT) == "false") {
-      false
-    } else {
-      throw QueryExecutionErrors.paramIsNotBooleanValueError(paramName)
+    val paramValue = parameters.get(paramName)
+    paramValue match {
+      case None => default
+      case Some(null) => default
+      case Some(value) => value.toLowerCase(Locale.ROOT) match {
+        case "true" => true
+        case "false" => false
+        case _ => throw QueryExecutionErrors.paramIsNotBooleanValueError(paramName)
+      }
     }
   }
 
diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/xml/XmlOptions.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/xml/XmlOptions.scala
index bd4da3b27c86..49c82b2c0e56 100644
--- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/xml/XmlOptions.scala
+++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/xml/XmlOptions.scala
@@ -78,15 +78,15 @@ class XmlOptions(
   }
 
   private def getBool(paramName: String, default: Boolean = false): Boolean = {
-    val param = parameters.getOrElse(paramName, default.toString)
-    if (param == null) {
-      default
-    } else if (param.toLowerCase(Locale.ROOT) == "true") {
-      true
-    } else if (param.toLowerCase(Locale.ROOT) == "false") {
-      false
-    } else {
-      throw QueryExecutionErrors.paramIsNotBooleanValueError(paramName)
+    val paramValue = parameters.get(paramName)
+    paramValue match {
+      case None => default
+      case Some(null) => default
+      case Some(value) => value.toLowerCase(Locale.ROOT) match {
+        case "true" => true
+        case "false" => false
+        case _ => throw QueryExecutionErrors.paramIsNotBooleanValueError(paramName)
+      }
     }
   }
 


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