This is an automated email from the ASF dual-hosted git repository.
morningman pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new 6c27d4d8b0b [fix](nereids) Do not turn invalid UTF-8 from BE constant
folding into U+FFFD (#69143)
6c27d4d8b0b is described below
commit 6c27d4d8b0b4649f6517e15792fba672a54e2935
Author: Mingyu Chen (Rayner) <[email protected]>
AuthorDate: Sat Oct 17 07:27:20 2026 +0100
[fix](nereids) Do not turn invalid UTF-8 from BE constant folding into
U+FFFD (#69143)
### What problem does this PR solve?
Issue Number: None
Related PR: #69025
Problem Summary:
**In short.** With `enable_fold_constant_by_be=true`, FE turned the
bytes of a string that BE computed for a constant into a literal by
decoding them as UTF-8 with replacement, so bytes that are not valid
UTF-8 became U+FFFD (`EF BF BD`). `WHERE s = unhex('FF')` then matched
the rows holding U+FFFD instead of the rows holding the byte `FF`, and
`SELECT unhex('FF')` returned `EF BF BD`. This PR leaves such a constant
unfolded, so BE evaluates it as it does with the variable off. Strings
that are valid UTF-8 fold as before. This was found while testing
#69025, which keeps its VARBINARY casts away from BE folding partly for
this reason.
**Background.** With `enable_fold_constant_by_be=true`,
`FoldConstantRuleOnBE` sends the largest constant subtrees of an
expression to a BE and replaces them with literals built from the values
BE returns (`getResultExpression`). BE writes the bytes of a string
value as they are into `PValues.string_value`, a proto2 `string` field
that does not require UTF-8, and FE read them with `getStringValue(i)`,
which decodes with replacement. A FE string literal is a Java string, so
it holds only text that is valid UTF-8. FE has no rule of its own for
`unhex`, so with the variable off the call stays in the plan and BE
evaluates it on the real bytes. The variable is off by default;
regression pipelines turn it on at random.
**The problem, and what it cost.** A table `t(k, s)` holds `'a'` (1),
`FF` (2), U+FFFD, `EFBFBD` (3), `61FF` (4), NULL (5) and `'é'`, `C3A9`
(6):
| Query | BE folding off, or `debug_skip_fold_constant=true` | master,
`enable_fold_constant_by_be=true` | with this PR |
|---|---|---|---|
| `SELECT k FROM t WHERE s = unhex('FF')` | 2 | **3** | 2 |
| `... WHERE s = concat('a', unhex('FF'))` | 4 | **no row** | 4 |
| `... WHERE array_contains(array(unhex('FF')), s)`, and the same with a
MAP key or a STRUCT field | 2 | **3** | 2 |
| `SELECT hex(concat(s, unhex('FF'))) FROM t WHERE k = 1` | `61FF` |
**`61EFBFBD`** | `61FF` |
| `SELECT unhex('FF')`, bytes sent to the client | `FF` | **`EF BF BD`**
| `FF` |
| `EXPLAIN ... WHERE s = unhex('FF')` | `s = unhex('FF')` | `s = '�'` |
`s = unhex('FF')` |
| `EXPLAIN ... WHERE s = unhex('C3A9')` | `s = unhex('C3A9')` | `s =
'é'` | `s = 'é'` |
The query succeeds either way, so the wrong rows and bytes go unnoticed:
a session variable changes which rows a filter on binary data matches.
**How this PR fixes it.** For a string type, `getResultExpression` reads
the bytes (`getStringValueBytes(i)`) and builds the literal only when
`ByteString.isValidUtf8()` holds; otherwise it returns no literal, which
`evalOnBE` already treats as "keep the expression" (as for a NaN float).
That check rejects exactly the bytes a Java string cannot carry: stray
bytes, truncated sequences, overlong forms, surrogates and code points
beyond U+10FFFF. An array, map or struct holding such a string is not
folded either, since those branches already stop when an element is not
folded. A valid string decodes to the same text as before, NUL bytes and
supplementary characters included.
The JSON branch is unchanged: BE never folds a JSON expression
(`shouldSkipFold`), and a JSON value inside a folded array, map or
struct can only come from a CAST that parses text, and BE's parser
rejects text that is not valid UTF-8 (`CAST(array(unhex('22FF22')) AS
ARRAY<JSON>)` is `[null]` with and without BE folding).
**Results.**
- **FE UT.**
`LiteralTest.testGetResultExpressionLeavesInvalidUtf8Unfolded`: `FF`,
`61FF`, `C3`, `C080` and `EDA080` are not folded as STRING, VARCHAR or
CHAR, nor as an ARRAY element, a MAP key or value, or a STRUCT field;
the same results with `C3A9` fold.
`testGetResultExpressionStringKeepsItsBytes`: valid UTF-8 with a NUL
byte, U+1F600 or U+FFFD itself folds to the same bytes. Without the fix
the first test fails. All tests in `LiteralTest` pass.
- **Regression.** The new `fold_constant_by_be_invalid_utf8`
(query_p0/expression/fold_constant) runs the filters and projections
above with FE folding, with BE folding and with folding skipped; the
results of each block must match in the three modes, and two `EXPLAIN`
checks show that `unhex('C3A9')` is still folded on BE and `unhex('FF')`
is not. Its `.out` was generated on a local cluster with this change;
with the FE before this change the suite fails at `filter_fold_by_be`
(row 3 instead of 2).
- **End to end.** The table above, on the same local cluster before and
after this change.
### Release note
With `enable_fold_constant_by_be=true`, a constant string whose bytes
are not valid UTF-8, such as `unhex('FF')`, keeps its bytes instead of
turning into U+FFFD, so filters and results match those with the
variable off.
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [x] Regression test
- `fold_constant_by_be_invalid_utf8`
(query_p0/expression/fold_constant): filters and projections on
`unhex('FF')` and the strings, arrays, maps and structs built from it,
in three folding modes.
- [x] Unit Test
- `LiteralTest.testGetResultExpressionLeavesInvalidUtf8Unfolded` and
`LiteralTest.testGetResultExpressionStringKeepsItsBytes`.
- [x] Manual test (add detailed scripts or steps below)
- The queries above on a local cluster, before and after this change.
- [ ] No need to test or manual test. Explain why:
- [ ] This is a refactor/code format and no logic has been changed.
- [ ] Previous test can cover this change.
- [ ] No code files have been changed.
- [ ] Other reason <!-- Add your reason? -->
- Behavior changed:
- [ ] No.
- [x] Yes. <!-- Explain the behavior change -->
- With `enable_fold_constant_by_be=true`, a constant whose string result
is not valid UTF-8 is no longer folded; BE evaluates it, and FE logs the
existing `Be constant folding convert ... failed` warning.
- Does this need documentation?
- [x] No.
- [ ] Yes. <!-- Add document PR link here. eg:
https://github.com/apache/doris-website/pull/1214 -->
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
Co-authored-by: Claude Opus 5.5 (1M context) <[email protected]>
---
.../expression/rules/FoldConstantRuleOnBE.java | 9 +-
.../nereids/trees/expressions/LiteralTest.java | 109 +++++++++++++++++++++
.../fold_constant_by_be_invalid_utf8.out | 57 +++++++++++
.../fold_constant_by_be_invalid_utf8.groovy | 77 +++++++++++++++
4 files changed, 251 insertions(+), 1 deletion(-)
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/FoldConstantRuleOnBE.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/FoldConstantRuleOnBE.java
index d8fd83457c8..57a5a5fe1d9 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/FoldConstantRuleOnBE.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/FoldConstantRuleOnBE.java
@@ -562,7 +562,14 @@ public class FoldConstantRuleOnBE implements
ExpressionPatternRuleFactory {
} else if (type.isStringLikeType()) {
int num = resultContent.getStringValueCount();
for (int i = 0; i < num; ++i) {
- Literal literal = new
StringLiteral(resultContent.getStringValue(i));
+ // BE sends the bytes of a string as they are, but a FE string
literal holds only valid UTF-8:
+ // getStringValue(i) would turn other bytes, such as the 0xFF
of unhex('FF'), into U+FFFD.
+ // Leave such a result unfolded, so that BE evaluates the
expression.
+ ByteString bytesValue = resultContent.getStringValueBytes(i);
+ if (!bytesValue.isValidUtf8()) {
+ return Collections.emptyList();
+ }
+ Literal literal = new StringLiteral(bytesValue.toStringUtf8());
res.add(literal);
}
} else if (type.isArrayType()) {
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/expressions/LiteralTest.java
b/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/expressions/LiteralTest.java
index aaa63ac3562..681c6d2daff 100644
---
a/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/expressions/LiteralTest.java
+++
b/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/expressions/LiteralTest.java
@@ -17,30 +17,40 @@
package org.apache.doris.nereids.trees.expressions;
+import org.apache.doris.common.Pair;
import org.apache.doris.nereids.trees.expressions.literal.ArrayLiteral;
import org.apache.doris.nereids.trees.expressions.literal.BooleanLiteral;
import org.apache.doris.nereids.trees.expressions.literal.IntegerLiteral;
import org.apache.doris.nereids.trees.expressions.literal.Literal;
+import org.apache.doris.nereids.trees.expressions.literal.MapLiteral;
import org.apache.doris.nereids.trees.expressions.literal.NullLiteral;
import org.apache.doris.nereids.trees.expressions.literal.StringLiteral;
+import org.apache.doris.nereids.trees.expressions.literal.StructLiteral;
import org.apache.doris.nereids.trees.expressions.literal.TimeStampNsLiteral;
import org.apache.doris.nereids.types.ArrayType;
+import org.apache.doris.nereids.types.CharType;
import org.apache.doris.nereids.types.DataType;
import org.apache.doris.nereids.types.DateTimeV2Type;
import org.apache.doris.nereids.types.IntegerType;
+import org.apache.doris.nereids.types.MapType;
import org.apache.doris.nereids.types.StringType;
import org.apache.doris.nereids.types.StructField;
import org.apache.doris.nereids.types.StructType;
import org.apache.doris.nereids.types.TimeStampNsType;
+import org.apache.doris.nereids.types.VarcharType;
import org.apache.doris.proto.Types.PGenericType;
import org.apache.doris.proto.Types.PGenericType.TypeId;
import org.apache.doris.proto.Types.PValues;
+import com.google.common.collect.ImmutableList;
+import com.google.protobuf.ByteString;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
+import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.Arrays;
+import java.util.HexFormat;
import java.util.List;
class LiteralTest {
@@ -396,4 +406,103 @@ class LiteralTest {
// STRUCT('col1':[0, 2, 4, 6, 8],'col2':['str1', 'str3', 'str5',
'str7', 'str9'])
System.out.println(resultExpression.get(0).toString());
}
+
+ @Test
+ public void testGetResultExpressionStringKeepsItsBytes() {
+ // BE sends the bytes of a string result as they are. Valid UTF-8,
including a NUL byte, a supplementary
+ // character (U+1F600) and U+FFFD itself, folds to a literal with the
same bytes.
+ List<String> hexes = ImmutableList.of("", "61", "C3A9", "610062",
"F09F9880", "EFBFBD");
+ List<Literal> result =
org.apache.doris.nereids.rules.expression.rules.FoldConstantRuleOnBE
+ .getResultExpression(StringType.INSTANCE, stringValues(hexes));
+ Assertions.assertEquals(hexes, stringLeaves(result));
+ }
+
+ @Test
+ public void testGetResultExpressionLeavesInvalidUtf8Unfolded() {
+ // A FE string literal cannot hold these bytes: 0xFF (unhex('FF')), a
truncated sequence, an overlong NUL
+ // and a UTF-16 surrogate. Decoding them would give U+FFFD, so neither
the string nor an array, a map or
+ // a struct holding it is folded, and BE evaluates the expression.
+ for (String invalid : ImmutableList.of("FF", "61FF", "C3", "C080",
"EDA080")) {
+ for (Pair<DataType, PValues> result : stringResults(invalid)) {
+ Assertions.assertEquals(ImmutableList.of(),
org.apache.doris.nereids.rules.expression.rules
+
.FoldConstantRuleOnBE.getResultExpression(result.first, result.second),
+ invalid + " in " + result.first);
+ }
+ }
+ // The same results fold when the bytes are valid UTF-8, here 'é'
(C3A9).
+ for (Pair<DataType, PValues> result : stringResults("C3A9")) {
+ Assertions.assertEquals(ImmutableList.of("61", "C3A9"),
stringLeaves(org.apache.doris.nereids.rules
+
.expression.rules.FoldConstantRuleOnBE.getResultExpression(result.first,
result.second)),
+ result.first.toString());
+ }
+ }
+
+ // Results of each shape that hold the strings 'a' and the bytes of
secondHex, laid out as BE writes them.
+ private static List<Pair<DataType, PValues>> stringResults(String
secondHex) {
+ PValues strings = stringValues(ImmutableList.of("61", secondHex));
+ PValues ints = PValues.newBuilder()
+ .setType(PGenericType.newBuilder().setId(TypeId.INT32).build())
+ .addInt32Value(1)
+ .addInt32Value(2)
+ .build();
+ PValues array = PValues.newBuilder()
+ .setType(PGenericType.newBuilder().setId(TypeId.LIST).build())
+ .addChildElement(strings)
+ .addChildOffset(2)
+ .build();
+ PValues stringKeyMap = PValues.newBuilder()
+ .setType(PGenericType.newBuilder().setId(TypeId.MAP).build())
+ .addChildElement(strings)
+ .addChildElement(ints)
+ .addChildOffset(2)
+ .build();
+ PValues stringValueMap = PValues.newBuilder()
+ .setType(PGenericType.newBuilder().setId(TypeId.MAP).build())
+ .addChildElement(ints)
+ .addChildElement(strings)
+ .addChildOffset(2)
+ .build();
+ PValues struct = PValues.newBuilder()
+
.setType(PGenericType.newBuilder().setId(TypeId.STRUCT).build())
+ .addChildElement(ints)
+ .addChildElement(strings)
+ .build();
+ StructType structType = new StructType(ImmutableList.of(
+ new StructField("i", IntegerType.INSTANCE, true, ""),
+ new StructField("s", StringType.INSTANCE, true, "")));
+ return ImmutableList.of(
+ Pair.of(StringType.INSTANCE, strings),
+ Pair.of(VarcharType.createVarcharType(10), strings),
+ Pair.of(CharType.createCharType(10), strings),
+ Pair.of(ArrayType.of(StringType.INSTANCE), array),
+ Pair.of(MapType.of(StringType.INSTANCE, IntegerType.INSTANCE),
stringKeyMap),
+ Pair.of(MapType.of(IntegerType.INSTANCE, StringType.INSTANCE),
stringValueMap),
+ Pair.of(structType, struct));
+ }
+
+ private static PValues stringValues(List<String> hexes) {
+ PValues.Builder builder = PValues.newBuilder()
+
.setType(PGenericType.newBuilder().setId(TypeId.STRING).build());
+ hexes.forEach(hex ->
builder.addStringValueBytes(ByteString.copyFrom(HexFormat.of().parseHex(hex))));
+ return builder.build();
+ }
+
+ // The UTF-8 bytes, in hex, of the string literals in the results, in the
order the results hold them.
+ private static List<String> stringLeaves(List<? extends Literal> literals)
{
+ List<String> leaves = new ArrayList<>();
+ for (Literal literal : literals) {
+ if (literal instanceof StringLiteral) {
+ leaves.add(HexFormat.of().withUpperCase()
+ .formatHex(((StringLiteral)
literal).getValue().getBytes(StandardCharsets.UTF_8)));
+ } else if (literal instanceof ArrayLiteral) {
+ leaves.addAll(stringLeaves(((ArrayLiteral)
literal).getValue()));
+ } else if (literal instanceof StructLiteral) {
+ leaves.addAll(stringLeaves(((StructLiteral)
literal).getValue()));
+ } else if (literal instanceof MapLiteral) {
+ ((MapLiteral) literal).getValue().forEach((key, value) ->
+ leaves.addAll(stringLeaves(ImmutableList.of(key,
value))));
+ }
+ }
+ return leaves;
+ }
}
diff --git
a/regression-test/data/query_p0/expression/fold_constant/fold_constant_by_be_invalid_utf8.out
b/regression-test/data/query_p0/expression/fold_constant/fold_constant_by_be_invalid_utf8.out
new file mode 100644
index 00000000000..ab13d2543a5
--- /dev/null
+++
b/regression-test/data/query_p0/expression/fold_constant/fold_constant_by_be_invalid_utf8.out
@@ -0,0 +1,57 @@
+-- This file is automatically generated. You should know what you did if you
want to edit this
+-- !data --
+1 61
+2 FF
+3 EFBFBD
+4 61FF
+5 \N
+6 C3A9
+
+-- !filter_fold --
+array 2
+concat 4
+ff 2
+map 2
+struct 2
+utf8 6
+
+-- !project_fold --
+1 61FF 61C3A9
+2 FFFF FFC3A9
+3 EFBFBDFF EFBFBDC3A9
+4 61FFFF 61FFC3A9
+5 \N \N
+6 C3A9FF C3A9C3A9
+
+-- !filter_fold_by_be --
+array 2
+concat 4
+ff 2
+map 2
+struct 2
+utf8 6
+
+-- !project_fold_by_be --
+1 61FF 61C3A9
+2 FFFF FFC3A9
+3 EFBFBDFF EFBFBDC3A9
+4 61FFFF 61FFC3A9
+5 \N \N
+6 C3A9FF C3A9C3A9
+
+-- !filter_no_fold --
+array 2
+concat 4
+ff 2
+map 2
+struct 2
+utf8 6
+
+-- !project_no_fold --
+1 61FF 61C3A9
+2 FFFF FFC3A9
+3 EFBFBDFF EFBFBDC3A9
+4 61FFFF 61FFC3A9
+5 \N \N
+6 C3A9FF C3A9C3A9
+
diff --git
a/regression-test/suites/query_p0/expression/fold_constant/fold_constant_by_be_invalid_utf8.groovy
b/regression-test/suites/query_p0/expression/fold_constant/fold_constant_by_be_invalid_utf8.groovy
new file mode 100644
index 00000000000..7e0ff898a45
--- /dev/null
+++
b/regression-test/suites/query_p0/expression/fold_constant/fold_constant_by_be_invalid_utf8.groovy
@@ -0,0 +1,77 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+// BE folds a constant string to its bytes, and FE can build a string literal
from them only when they are
+// valid UTF-8. unhex('FF'), and the strings, arrays, maps and structs built
from it, hold the byte 0xFF, which a
+// FE literal would turn into U+FFFD (EFBFBD). So they must give the same
results whether FE folds the
+// constants (fold), FE and BE fold them (fold_by_be) or BE evaluates them
(no_fold); the results of a block
+// must match in the three modes. BE still folds a string that is valid UTF-8,
such as unhex('C3A9') ('é').
+// Each mode puts its name in a column alias, so that it is not served the
cached result of another mode.
+
+suite("fold_constant_by_be_invalid_utf8") {
+ sql "drop table if exists fold_constant_by_be_invalid_utf8_t"
+ sql """
+ CREATE TABLE fold_constant_by_be_invalid_utf8_t (k int, s string NULL)
+ DUPLICATE KEY(k) DISTRIBUTED BY HASH(k) BUCKETS 1 PROPERTIES
('replication_num' = '1')
+ """
+ // BE evaluates unhex, so the rows hold the bytes: 'a', 0xFF, U+FFFD, 'a'
0xFF, NULL and 'é'.
+ sql "set enable_fold_constant_by_be = false"
+ sql "set debug_skip_fold_constant = true"
+ sql """
+ INSERT INTO fold_constant_by_be_invalid_utf8_t VALUES
+ (1, 'a'), (2, unhex('FF')), (3, unhex('EFBFBD')), (4,
unhex('61FF')), (5, NULL), (6, unhex('C3A9'))
+ """
+ order_qt_data "SELECT k, hex(s) FROM fold_constant_by_be_invalid_utf8_t"
+
+ def modes = [
+ fold : [false, false],
+ fold_by_be: [true, false],
+ no_fold : [false, true]]
+ modes.each { mode, settings ->
+ sql "set enable_fold_constant_by_be = ${settings[0]}"
+ sql "set debug_skip_fold_constant = ${settings[1]}"
+ "order_qt_filter_${mode}" """
+ SELECT 'ff' AS filter_${mode}, k FROM
fold_constant_by_be_invalid_utf8_t WHERE s = unhex('FF')
+ UNION ALL
+ SELECT 'concat', k FROM fold_constant_by_be_invalid_utf8_t WHERE s
= concat('a', unhex('FF'))
+ UNION ALL
+ SELECT 'array', k FROM fold_constant_by_be_invalid_utf8_t WHERE
array_contains(array(unhex('FF')), s)
+ UNION ALL
+ SELECT 'map', k FROM fold_constant_by_be_invalid_utf8_t WHERE
map_contains_key(map(unhex('FF'), 1), s)
+ UNION ALL
+ SELECT 'struct', k FROM fold_constant_by_be_invalid_utf8_t
+ WHERE struct_element(if(k > 0, named_struct('f', unhex('FF')),
named_struct('f', 'x')), 'f') = s
+ UNION ALL
+ SELECT 'utf8', k FROM fold_constant_by_be_invalid_utf8_t WHERE s =
unhex('C3A9')
+ """
+ "order_qt_project_${mode}" """
+ SELECT k AS project_${mode}, hex(concat(s, unhex('FF'))),
hex(concat(s, unhex('C3A9')))
+ FROM fold_constant_by_be_invalid_utf8_t
+ """
+ }
+
+ sql "set enable_fold_constant_by_be = true"
+ sql "set debug_skip_fold_constant = false"
+ explain {
+ sql "SELECT k FROM fold_constant_by_be_invalid_utf8_t WHERE s =
unhex('C3A9')"
+ contains "= 'é')"
+ }
+ explain {
+ sql "SELECT k FROM fold_constant_by_be_invalid_utf8_t WHERE s =
unhex('FF')"
+ contains "= unhex('FF'))"
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
