Raviston6296 commented on PR #60054:
URL: https://github.com/apache/spark/pull/60054#issuecomment-6604852209
Thanks for the fix. The approach looks right to me. I confirmed the root
cause with plain Java: `Pattern.compile("(?ui).*i.*").matcher("ı").matches()`
is `true`, while `"ı".toLowerCase(Locale.ROOT)` is not `"i"`. The same goes for
the long s (U+017F) against `s`. Since the mismatch depends on the input value
and not only on the pattern, skipping the rewrite for all non-binary inputs
seems like the right minimal fix. `StringTypeBinaryLcase` only admits
UTF8_BINARY and UTF8_LCASE (no trim collations), so in practice this only
affects UTF8_LCASE.
A few small comments:
1. In `CollationSQLRegexpSuite`, `SELECT like(collate('ı', 'UTF8_LCASE'),
'%i%')` has only literal inputs. I believe `ConstantFolding` runs before
`LikeSimplification` in the same batch, so this query gets folded before the
rule can rewrite it, and it would pass even without the fix. The table-based
part below it is the real regression test. Consider dropping the literal query
or adding a comment that it isn't meant to cover the rewrite.
2. `input.dataType.asInstanceOf[StringType]` is safe today because `Like`
and `MultiLikeBase` require `StringTypeBinaryLcase`. A pattern match such as
`case st: StringType if st.supportsBinaryEquality =>` would avoid depending on
that. Minor, feel free to ignore.
3. This turns off the UTF8_LCASE part of SPARK-48621, so `c LIKE 'abc%'` on
a UTF8_LCASE column goes back to regex matching and will be slower. That
trade-off seems right for correctness, but it might be worth stating in the
"user-facing change" section.
4. Not for this PR: LIKE under UTF8_LCASE also disagrees with `=` under
UTF8_LCASE (`'ı' = 'i'` is false, but `'ı' LIKE 'i'` is true). Making LIKE use
the same lowercase mapping as other UTF8_LCASE operations would bring the
simplification back safely. That could be a follow-up JIRA.
--
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]
