morningman commented on code in PR #69025:
URL: https://github.com/apache/doris/pull/69025#discussion_r4259536008
##########
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:
Fixed in 0c522c34565 by leaving such casts to BE, not by changing how FE
evaluates strings.
The FE behavior is not specific to binary input. With a plain string
literal, `SELECT 'a\0b' = 'a'` also folds to true, because
`StringLikeLiteral.compareTo` takes the bytes after a NUL for padding, while BE
compares all three bytes. Before this PR the binary form folded to the hex text
`610062`, which happened to give BE's answer.
`VarBinaryLiteral.castValueToString()` now throws for decoded text with a NUL
byte or a supplementary character, as it already does for invalid UTF-8, so
`FoldConstantRuleOnFE.visitCast` leaves the cast to BE. Ordinary text still
folds.
`FoldConstantTest.testCastVarBinaryToStringFold` asserts that the cast of
`X'610062'` stays unfolded. `test_varbinary_cast_fold` has `edge_fold` /
`edge_no_fold` blocks with this comparison and the two expressions from the
other threads; both give `false`, `F09F9880`, `F09F9880`. With folding on, the
same expressions gave true, `3F`, `3F` before this commit.
##########
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:
Fixed in 0c522c34565, together with the NUL comparison (see that thread): a
cast whose decoded text has a supplementary character is now left to BE. The
truncation is not specific to binary input either: `CAST('😀' AS VARCHAR(1))`
with a plain literal folds to the lone high surrogate, which encodes as `3F`,
while BE keeps `F09F9880`. `FoldConstantTest` asserts that `CAST(X'F09F9880' AS
VARCHAR(1))` stays unfolded, and `test_varbinary_cast_fold` checks
`hex(CAST(X'F09F9880' AS VARCHAR(1)))` = `F09F9880` with folding on and off.
##########
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:
Fixed in 0c522c34565: the cast of `X'F09F9880'` is now left to BE, so
`trim_in` over it is no longer folded either. `StringArithmetic.trimInImpl`
does split surrogate pairs, and a plain literal reaches it too: `trim_in('😀',
'😁')` folds to `3F` while BE returns `F09F9880`. That trim-set code is shared
with `ltrim_in` / `rtrim_in` and is unchanged here. `test_varbinary_cast_fold`
checks `hex(trim_in(CAST(X'F09F9880' AS STRING), CAST(X'F09F9881' AS STRING)))`
= `F09F9880` with folding on and off.
--
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]
