[jira] [Commented] (SPARK-59043) SimplifyCaseConversionExpressions changes Unicode case-conversion semantics

["AnhTris (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/SPARK-59043?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18109369#comment-18109369 ] 

AnhTris commented on SPARK-59043:
---------------------------------

I've been working on this issue and have submitted a PR with comprehensive test suites to fix it: https://github.com/apache/spark/pull/58376

h3. Technical Investigation & Analysis

I have investigated this issue and verified the unexpected behavior across Apache Spark optimizer configurations. Below are the minimal reproduction steps, the identified root cause in Catalyst Optimizer, and the proposed fix.

----

h3. 1. Reproduction

The issue can be reproduced directly via Spark SQL ({{spark-sql}} CLI or {{spark.sql(...)}} in PySpark/Scala):

{code:sql}
-- 1. Query with default optimizer configuration:
SELECT lower(upper(s2)) AS result
FROM (VALUES ('ı')) AS t(s)
LATERAL VIEW explode(array(s)) e AS s2;
{code}

*Actual Output (Default optimizer configuration):*
{noformat}
+------+
|result|
+------+
|ı     |
+------+
{noformat}

*Output when excluding SimplifyCaseConversionExpressions:*
{code:sql}
SET spark.sql.optimizer.excludedRules=org.apache.spark.sql.catalyst.optimizer.SimplifyCaseConversionExpressions;

SELECT lower(upper(s2)) AS result
FROM (VALUES ('ı')) AS t(s)
LATERAL VIEW explode(array(s)) e AS s2;
{code}
{noformat}
+------+
|result|
+------+
|i     |
+------+
{noformat}

*Expected Output:*
Both configurations must preserve original Unicode semantics and return {{'i'}}:
* For character {{ı}} (U+0131 LATIN SMALL LETTER DOTLESS I):
** {{upper('ı')}} = {{'I'}} (U+0049 LATIN CAPITAL LETTER I)
** {{lower('I')}} = {{'i'}} (U+0069 LATIN SMALL LETTER I)
** Therefore, {{lower(upper('ı'))}} = {{'i'}}.

----

h3. 2. Root Cause

In Catalyst Optimizer ({{sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala}}):

{code:scala}
object SimplifyCaseConversionExpressions extends Rule[LogicalPlan] {
  def apply(plan: LogicalPlan): LogicalPlan = plan.transformWithPruning(
    _.containsPattern(UPPER_OR_LOWER), ruleId) {
    case q: LogicalPlan => q.transformExpressionsUpWithPruning(
      _.containsPattern(UPPER_OR_LOWER), ruleId) {
      case Upper(Upper(child)) => Upper(child)
      case Upper(Lower(child)) => Upper(child)  // <-- Incorrect Unicode assumption
      case Lower(Upper(child)) => Lower(child)  // <-- Incorrect Unicode assumption
      case Lower(Lower(child)) => Lower(child)
    }
  }
}
{code}

# The rule erroneously assumes that cross-case conversions {{Lower(Upper(child))}} and {{Upper(Lower(child))}} can be simplified to {{Lower(child)}} and {{Upper(child)}} respectively.
# In the Unicode Standard and Java / ICU runtime case mappings ({{UTF8String}}), mixed case conversion is non-symmetric and non-invertible for non-ASCII Unicode characters:
#* For {{ı}} (U+0131): {{lower(upper('ı'))}} = {{'i'}} != {{lower('ı')}} ({{'ı'}}).
#* For {{µ}} (U+00B5): {{lower(upper('µ'))}} = {{'μ'}} != {{lower('µ')}} ({{'µ'}}).
#* For {{ß}} (U+00DF): {{lower(upper('ß'))}} = {{'ss'}} != {{lower('ß')}} ({{'ß'}}).
#* For {{ſ}} (U+017F): {{lower(upper('ſ'))}} = {{'s'}} != {{lower('ſ')}} ({{'ſ'}}).
#* For {{K}} (U+212A): {{upper(lower('K'))}} = {{'K'}} != {{upper('K')}} ({{'K'}}).
#* For {{Å}} (U+212B): {{upper(lower('Å'))}} = {{'Å'}} != {{upper('Å')}} ({{'Å'}}).
# Across the entire Unicode character set, 125 code points violate {{lower(upper(c)) == lower(c)}} and 6 code points violate {{upper(lower(c)) == upper(c)}}.
# Therefore, collapsing mixed case expressions alters the execution semantics of the query.

----

h3. 3. Proposed Fix

We remove the cross-case conversions from {{SimplifyCaseConversionExpressions}}, retaining only same-case idempotent simplifications ({{Upper(Upper(child))}} and {{Lower(Lower(child))}}):

{code:scala}
object SimplifyCaseConversionExpressions extends Rule[LogicalPlan] {
  def apply(plan: LogicalPlan): LogicalPlan = plan.transformWithPruning(
    _.containsPattern(UPPER_OR_LOWER), ruleId) {
    case q: LogicalPlan => q.transformExpressionsUpWithPruning(
      _.containsPattern(UPPER_OR_LOWER), ruleId) {
      case Upper(Upper(child)) => Upper(child)
      case Lower(Lower(child)) => Lower(child)
    }
  }
}
{code}

Additionally:
# Updated {{SimplifyStringCaseConversionSuite.scala}} to verify that {{Upper(Lower(str))}} and {{Lower(Upper(str))}} are preserved in the analyzed logical plan.
# Added regression tests in {{StringFunctionsSuite.scala}} covering Unicode characters ({{'ı'}}, {{'µ'}}, {{'ß'}}, {{'K'}}) to verify semantic parity with and without optimizer rule exclusion.
# Updated {{docs/sql-migration-guide.md}} documenting this optimizer behavior change.


> SimplifyCaseConversionExpressions changes Unicode case-conversion semantics
> ---------------------------------------------------------------------------
>
>                 Key: SPARK-59043
>                 URL: https://issues.apache.org/jira/browse/SPARK-59043
>             Project: Spark
>          Issue Type: Bug
>          Components: SQL
>    Affects Versions: 5.0.0
>         Environment: Spark 5.0.0-SNAPSHOT, commit 9da9f8d673914d1648514f59d85e3adafb300d1a; macOS arm64; Java 17.0.17
>            Reporter: Yibo Dong
>            Priority: Major
>              Labels: optimizer, pull-request-available, sql
>
> A legal nested Unicode case-conversion expression returns different results
> depending on whether SimplifyCaseConversionExpressions is enabled.
> h3. Reproduction
> {code:sql}
> SELECT lower(upper(s2)) AS result
> FROM (VALUES ('ı')) AS t(s)
> LATERAL VIEW explode(array(s)) e AS s2;
> {code}
> With the default optimizer configuration, the query returns:
> {code}
> ı
> {code}
> If only SimplifyCaseConversionExpressions is excluded:
> {code}
> --conf spark.sql.optimizer.excludedRules=org.apache.spark.sql.catalyst.optimizer.SimplifyCaseConversionExpressions
> {code}
> the same query returns:
> {code}
> i
> {code}
> h3. Expected behavior
> Both configurations should preserve the semantics of the original expression and
> return:
> {code}
> i
> {code}
> For the Unicode character U+0131 LATIN SMALL LETTER DOTLESS I:
> {code}
> upper('ı') = 'I'
> lower('I') = 'i'
> {code}
> Therefore:
> {code}
> lower(upper('ı')) = 'i'
> {code}
> h3. Actual behavior
> With SimplifyCaseConversionExpressions enabled, Spark returns `ı`.
> The optimizer rule simplifies:
> {code}
> lower(upper(child))
> {code}
> to:
> {code}
> lower(child)
> {code}
> However, this transformation is not semantics-preserving for all Unicode
> characters. For this input:
> {code}
> lower(upper('ı')) = 'i'
> lower('ı') = 'ı'
> {code}
> h3. Additional information
> The only configuration change needed to avoid the incorrect result is excluding
> SimplifyCaseConversionExpressions.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

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