github-actions[bot] commented on code in PR #69025:
URL: https://github.com/apache/doris/pull/69025#discussion_r4260479286
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/literal/VarBinaryLiteral.java:
##########
@@ -73,8 +79,39 @@ 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).
+ *
+ * <p>Valid UTF-8 with a NUL byte or a supplementary character is left for
BE too, because FE does not
+ * evaluate such string literals like BE does: StringLikeLiteral.compareTo
takes the bytes after a NUL for
+ * padding, so CAST(X'610062' AS STRING) = 'a' would fold to TRUE while BE
compares all three bytes, and
+ * VARCHAR(n) truncation and trim_in work on UTF-16 units, so
CAST(X'F09F9880' AS VARCHAR(1)) would keep
+ * half of U+1F600 where BE keeps the whole character.</p>
+ */
+ @Override
protected String castValueToString() {
- return toString();
+ String value = toUtf8String().orElseThrow(() -> new AnalysisException(
+ "Cannot cast VARBINARY " + computeToSql() + " to a string type
on FE: it is not valid UTF-8"));
+ if (value.indexOf('\0') >= 0 ||
value.codePoints().anyMatch(Character::isSupplementaryCodePoint)) {
+ throw new AnalysisException("Cannot cast VARBINARY " +
computeToSql()
+ + " to a string type on FE: it has a NUL byte or a
supplementary character");
+ }
+ return value;
Review Comment:
[P2] Match BE semantics for bounded strings inside array casts
`SELECT CAST([X'6162'] AS ARRAY<VARCHAR(1)>)[1] = 'a'` now returns `TRUE`
with FE folding, whereas skipping folding returns `FALSE`. This uses the two
ASCII bytes `ab`, so the NUL/supplementary guards do not apply.
In the reduced `Project(ElementAt(Cast([X'6162'], ARRAY<VARCHAR(1)>), 1) =
'a')`, `ArrayLiteral.uncheckedCastTo` calls the element's checked cast. This
method returns `ab`, and `Literal.uncheckedCastToString` builds
`VarcharLiteral("ab", 1)`, replacing the array cast with `['a']`. On BE,
`create_array_wrapper` recursively invokes the string cast wrapper, which
preserves both bytes and does not enforce the nested VARCHAR length. The parser
adds `substr` only when the whole cast target is string-like, so no nested
substring narrows `['ab']`; extracting its element and comparing with `a`
returns false. The previous hex-text fold truncated `6162` to `6`, so this
predicate previously also returned false.
Defer these bounded recursive conversions or make FE element folding agree
with BE, and add folded/unfolded bounded-array coverage. This is a nested
length-semantics mismatch, distinct from the existing supplementary-character
splitting report.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/literal/VarBinaryLiteral.java:
##########
@@ -73,8 +79,39 @@ 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).
+ *
+ * <p>Valid UTF-8 with a NUL byte or a supplementary character is left for
BE too, because FE does not
+ * evaluate such string literals like BE does: StringLikeLiteral.compareTo
takes the bytes after a NUL for
+ * padding, so CAST(X'610062' AS STRING) = 'a' would fold to TRUE while BE
compares all three bytes, and
+ * VARCHAR(n) truncation and trim_in work on UTF-16 units, so
CAST(X'F09F9880' AS VARCHAR(1)) would keep
+ * half of U+1F600 where BE keeps the whole character.</p>
+ */
+ @Override
protected String castValueToString() {
- return toString();
+ String value = toUtf8String().orElseThrow(() -> new AnalysisException(
+ "Cannot cast VARBINARY " + computeToSql() + " to a string type
on FE: it is not valid UTF-8"));
+ if (value.indexOf('\0') >= 0 ||
value.codePoints().anyMatch(Character::isSupplementaryCodePoint)) {
Review Comment:
[P2] Allow deferred string constants in nullable IN subqueries
With the default `enable_fold_constant_by_be=false`, `SELECT CAST(X'610062'
AS STRING) IN (SELECT s FROM t2)` now fails with `varbinary type could not in
join equal conditions` when `t2.s` is nullable STRING. The comparison operands
are both STRING.
The reduced rewritten plan is:
```text
Project(mark)
MarkJoin(mark: EqualTo(Cast(X'610062', STRING), t2.s))
OneRowRelation(dummy)
Scan(t2: s nullable STRING)
```
This new guard retains the constant cast. `SubqueryToApply` keeps it as the
compare expression, and `InApplyToJoin` preserves the nullable equality as a
mark conjunct. `JoinUtils.isHashJoinCondition` rejects the slot-free constant
operand, so `PushDownExpressionsInHashCondition` leaves this non-hash mark
conjunct unchanged; the nested-loop projection rule also excludes mark joins.
`CheckAfterRewrite` then rejects the binary literal descendant inside the
STRING cast. Previously, folding to hex text removed that descendant and the
query passed this check.
Keep supported deferred STRING operands valid at this planner boundary, by
checking the actual compared types or projecting the constant expression to a
STRING slot, and cover a constant cast in a nullable `IN` subquery's SELECT
result.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/literal/VarBinaryLiteral.java:
##########
@@ -73,8 +79,39 @@ 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).
+ *
+ * <p>Valid UTF-8 with a NUL byte or a supplementary character is left for
BE too, because FE does not
+ * evaluate such string literals like BE does: StringLikeLiteral.compareTo
takes the bytes after a NUL for
+ * padding, so CAST(X'610062' AS STRING) = 'a' would fold to TRUE while BE
compares all three bytes, and
+ * VARCHAR(n) truncation and trim_in work on UTF-16 units, so
CAST(X'F09F9880' AS VARCHAR(1)) would keep
+ * half of U+1F600 where BE keeps the whole character.</p>
+ */
+ @Override
protected String castValueToString() {
- return toString();
+ String value = toUtf8String().orElseThrow(() -> new AnalysisException(
Review Comment:
[P2] Preserve invalid bytes through optional BE constant folding
With `enable_fold_constant_by_be=true`, `SELECT CAST(X'FF' AS STRING) = '�'`
is newly folded to `TRUE`; full BE evaluation returns `FALSE`. The right
operand is U+FFFD (`EFBFBD`), distinct from the input byte `FF`.
A reduced triggering plan is:
```text
Filter(EqualTo(Cast(X'FF', STRING), '�'))
Scan(t)
```
`visitCast` catches this `AnalysisException` and retains the cast, but the
bottom-up `FoldConstantRule` then applies `FoldConstantRuleOnBE` to that child.
BE returns raw `FF` through `PValues.string_value`; `getResultExpression`
creates `StringLiteral(getStringValue(i))`, and the [protobuf Java
getter](https://github.com/protocolbuffers/protobuf/blob/v33.4/java/core/src/main/java/com/google/protobuf/LazyStringArrayList.java#L89-L99)
substitutes U+FFFD for invalid UTF-8. Parent `visitEqualTo` consequently
produces `Filter(TRUE)`, admitting rows BE would reject. The previous hex-text
fold compared `'FF'` with U+FFFD and returned `FALSE`.
Keep unrepresentable casts out of BE constant replacement as well, or
validate the raw returned bytes before constructing an FE string literal. Add
predicate and byte-visible coverage with `enable_fold_constant_by_be` both on
and off; the new FE-only unit test does not exercise this result transport.
--
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]
