github-actions[bot] commented on code in PR #69025:
URL: https://github.com/apache/doris/pull/69025#discussion_r4258348122


##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/literal/VarBinaryLiteral.java:
##########
@@ -73,8 +79,28 @@ public Object getValue() {
         return byteValues;
     }
 
+    /**
+     * BE casts VARBINARY to a string type by keeping the bytes as they are, 
so CAST(X'616263' AS STRING) is
+     * 'abc', not the hex text '616263'. A Java string carries those bytes 
only when they are valid UTF-8; for
+     * other bytes, such as X'FF', this throws, and 
FoldConstantRuleOnFE.visitCast then leaves the cast for BE
+     * to evaluate (it turns only a CastException into NULL).
+     */
+    @Override
     protected String castValueToString() {
-        return toString();
+        return toUtf8String().orElseThrow(() -> new AnalysisException(

Review Comment:
   [P2] Keep supplementary characters intact in bounded VARCHAR folds
   
   `SELECT CAST(X'F09F9880' AS VARCHAR(1)) = '?'` now folds to `TRUE`. The 
decoder returns the intact emoji, but `Literal.uncheckedCastToString` 
constructs `VarcharLiteral(value, 1)`, whose UTF-16 `substring(0, 1)` retains 
only its high surrogate. The parser produces 
`Project(EqualTo(substr(Cast(X'F09F9880', VARCHAR(1)), 1, 1), '?'))`; the outer 
character-count `substr` cannot restore the discarded half, and 
`StringLikeLiteral.compareTo` encodes the lone surrogate as `?` (`3F`). BE 
retains the whole character before substring and returns `FALSE`, as did the 
old hex-text predicate (`'F' = '?'`).
   
   Defer this bounded cast or construct its literal without splitting code 
points, and cover a supplementary character at the length boundary with folding 
enabled and disabled.



##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/literal/VarBinaryLiteral.java:
##########
@@ -73,8 +79,28 @@ public Object getValue() {
         return byteValues;
     }
 
+    /**
+     * BE casts VARBINARY to a string type by keeping the bytes as they are, 
so CAST(X'616263' AS STRING) is
+     * 'abc', not the hex text '616263'. A Java string carries those bytes 
only when they are valid UTF-8; for
+     * other bytes, such as X'FF', this throws, and 
FoldConstantRuleOnFE.visitCast then leaves the cast for BE
+     * to evaluate (it turns only a CastException into NULL).
+     */
+    @Override
     protected String castValueToString() {
-        return toString();
+        return toUtf8String().orElseThrow(() -> new AnalysisException(
+                "Cannot cast VARBINARY " + computeToSql() + " to a string type 
on FE: it is not valid UTF-8"));
+    }
+
+    /** The bytes decoded as UTF-8, or empty when they are not valid UTF-8 and 
only BE can cast them. */
+    public Optional<String> toUtf8String() {
+        try {
+            return Optional.of(StandardCharsets.UTF_8.newDecoder()
+                    .onMalformedInput(CodingErrorAction.REPORT)
+                    .onUnmappableCharacter(CodingErrorAction.REPORT)
+                    .decode(ByteBuffer.wrap(byteValues)).toString());

Review Comment:
   [P2] Keep decoded characters whole during trim_in folding
   
   `SELECT trim_in(CAST(X'F09F9880' AS STRING), '😁') = '?'` newly folds to 
`TRUE`. The decoded 😀 and trim-set 😁 share a UTF-16 high surrogate. 
`StringArithmetic.trimInImpl` checks membership with `charAt`/`indexOf`, 
removes that matching half and leaves an unmatched low surrogate; UTF-8 
comparison turns it into `?`. BE `TrimInUtil` compares whole UTF-8 characters, 
leaves 😀 intact and returns `FALSE`, as did the old hex-text predicate.
   
   The reduced `Project(EqualTo(trim_in(Cast(binary, STRING), '😁'), '?'))` 
becomes `Project(TRUE)`. This uses an unbounded STRING cast, so guarding 
bounded VARCHAR targets alone does not fix it. Use code-point membership in the 
trim-set implementation or defer affected folding, and add folded/unfolded 
coverage for `trim_in` and its shared `ltrim_in`/`rtrim_in` paths.



##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/literal/VarBinaryLiteral.java:
##########
@@ -73,8 +79,28 @@ public Object getValue() {
         return byteValues;
     }
 
+    /**
+     * BE casts VARBINARY to a string type by keeping the bytes as they are, 
so CAST(X'616263' AS STRING) is
+     * 'abc', not the hex text '616263'. A Java string carries those bytes 
only when they are valid UTF-8; for
+     * other bytes, such as X'FF', this throws, and 
FoldConstantRuleOnFE.visitCast then leaves the cast for BE
+     * to evaluate (it turns only a CastException into NULL).
+     */
+    @Override
     protected String castValueToString() {
-        return toString();
+        return toUtf8String().orElseThrow(() -> new AnalysisException(
+                "Cannot cast VARBINARY " + computeToSql() + " to a string type 
on FE: it is not valid UTF-8"));
+    }
+
+    /** The bytes decoded as UTF-8, or empty when they are not valid UTF-8 and 
only BE can cast them. */
+    public Optional<String> toUtf8String() {
+        try {
+            return Optional.of(StandardCharsets.UTF_8.newDecoder()

Review Comment:
   [P1] Preserve BE comparison semantics for decoded NUL bytes
   
   `SELECT CAST(X'610062' AS STRING) = 'a'` now folds to `TRUE`. `visitCast` 
creates the string containing `a`, NUL, `b`, then `visitEqualTo` calls 
`StringLikeLiteral.compareTo`, which returns equality when the first unmatched 
byte is zero, even with significant bytes after it. BE preserves all three 
bytes and compares their explicit lengths, returning `FALSE`; the previous 
hex-text fold also returned `FALSE`.
   
   A reduced triggering plan is `Filter(EqualTo(Cast(X'610062', STRING), 'a')) 
-> Scan(t)`. `visitCast` followed by `visitEqualTo` rewrites its predicate to 
`TRUE`, admitting rows BE would reject. Defer NUL-bearing casts until FE 
comparisons match BE, or correct that comparator alongside this change, and add 
folded/unfolded comparison coverage.



-- 
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]

Reply via email to