sam-1112 commented on code in PR #5409:
URL: https://github.com/apache/datafusion-comet/pull/5409#discussion_r3834763719
##########
spark/src/main/scala/org/apache/comet/serde/strings.scala:
##########
@@ -178,29 +179,56 @@ object CometStringReplace
extends CometScalarFunction[StringReplace]("replace")
with NativeOptInAvailable {
+ /**
+ * Native DataFusion `replace` differs from Spark only when the search
string is empty (Spark
+ * returns `src` unchanged; DataFusion inserts the replacement between every
character). That
+ * case is decidable at plan time when `search` is a literal.
+ *
+ * The native kernel is also byte-level `UTF8_BINARY` only, so non-default
collations stay on
+ * the dispatcher. https://github.com/apache/datafusion-comet/issues/4496
+ */
+ private def nativeSafeSearchSubset(expr: StringReplace): Boolean = {
+ val children = expr.children
+ if (children.length != 3) {
+ return false
+ }
+ val searchIsNonEmptyLiteral = children(1) match {
+ case Literal(v: UTF8String, _) => v != null && v.numBytes() > 0
Review Comment:
Agreed — kernel compatibility is not enough if `CometLiteral` changes the
search bytes. `CAST(X'FF' AS STRING)` folds to a non-empty `UTF8String`, so the
old “non-empty literal” check accepted it. Serialization then went through
`UTF8String.toString` and the search became U+FFFD (`EF BF BD`), which
incorrectly matched a well-formed U+FFFD in the source. The default native-safe
subset now rejects a string literal unless its bytes survive that round-trip:
```scala
Arrays.equals(v.getBytes, v.toString.getBytes(StandardCharsets.UTF_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: [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]
