Copilot commented on code in PR #58376:
URL: https://github.com/apache/spark/pull/58376#discussion_r3878680574
##########
sql/core/src/test/scala/org/apache/spark/sql/StringFunctionsSuite.scala:
##########
@@ -1564,4 +1564,55 @@ class StringFunctionsSuite extends SharedSparkSession {
)
}
}
+
+ test("SPARK-59043: SimplifyCaseConversionExpressions preserves Unicode
case-conversion semantics") {
+ val excludedConf =
"org.apache.spark.sql.catalyst.optimizer.SimplifyCaseConversionExpressions"
+ Seq(true, false).foreach { optimizerEnabled =>
+ val confModifier = if (optimizerEnabled) {
+ Map.empty[String, String]
+ } else {
+ Map(SQLConf.OPTIMIZER_EXCLUDED_RULES.key -> excludedConf)
+ }
+
+ withSQLConf(confModifier.toSeq: _*) {
+ // 1. Turkish dotless i (U+0131 LATIN SMALL LETTER DOTLESS I)
+ checkAnswer(
+ sql("SELECT lower(upper('ı')) AS result"),
+ Row("i") :: Nil
+ )
+ checkAnswer(
+ sql("SELECT lower(upper(s)) FROM (VALUES ('ı')) AS t(s)"),
+ Row("i") :: Nil
+ )
+ checkAnswer(
+ sql("SELECT lower(upper(s2)) AS result FROM (VALUES ('ı')) AS t(s)
LATERAL VIEW explode(array(s)) e AS s2"),
Review Comment:
This SQL string exceeds Spark's 100-character line limit for Scala sources,
which can trip scalastyle/linters. Consider splitting the query string across
multiple lines (e.g., using a triple-quoted string + stripMargin) to keep line
lengths under control.
##########
sql/core/src/test/scala/org/apache/spark/sql/StringFunctionsSuite.scala:
##########
@@ -1564,4 +1564,55 @@ class StringFunctionsSuite extends SharedSparkSession {
)
}
}
+
+ test("SPARK-59043: SimplifyCaseConversionExpressions preserves Unicode
case-conversion semantics") {
+ val excludedConf =
"org.apache.spark.sql.catalyst.optimizer.SimplifyCaseConversionExpressions"
+ Seq(true, false).foreach { optimizerEnabled =>
+ val confModifier = if (optimizerEnabled) {
+ Map.empty[String, String]
+ } else {
+ Map(SQLConf.OPTIMIZER_EXCLUDED_RULES.key -> excludedConf)
+ }
+
+ withSQLConf(confModifier.toSeq: _*) {
+ // 1. Turkish dotless i (U+0131 LATIN SMALL LETTER DOTLESS I)
+ checkAnswer(
Review Comment:
This new test introduces non-ASCII literals (e.g. 'ı', 'µ', 'ß', 'K'), but
this file generally requires disabling scalastyle around such sections (see
existing tests earlier in this suite). Without doing so, the non-ASCII style
check is likely to fail.
This issue also appears on line 1609 of the same file.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala:
##########
@@ -1184,17 +1184,19 @@ object SimplifyCasts extends Rule[LogicalPlan] {
/**
- * Removes the inner case conversion expressions that are unnecessary because
- * the inner conversion is overwritten by the outer one.
+ * Removes redundant same-case conversion expressions (e.g. UPPER(UPPER(x)) or
LOWER(LOWER(x)))
+ * that are unnecessary because the case conversion operation is idempotent.
+ *
+ * Note: Cross-case conversions (UPPER(LOWER(x)) or LOWER(UPPER(x))) are NOT
simplified
+ * because they are not semantics-preserving for non-ASCII Unicode characters
(e.g. Turkish
+ * dotless 'ı' where LOWER(UPPER('ı')) = 'i' != LOWER('ı')).
Review Comment:
This Scaladoc example includes non-ASCII characters (e.g. 'ı'). Spark
commonly keeps source comments ASCII-only to avoid triggering non-ASCII style
checks; consider rewriting the example to use U+XXXX code point notation
instead of embedding the character directly.
--
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]
