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]