This is an automated email from the ASF dual-hosted git repository.
yiguolei pushed a commit to branch branch-4.1
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/branch-4.1 by this push:
new 4926a55b6ac [fix](be) Fix null-safe string joins in 4.1 (backport
#65975) (#68809)
4926a55b6ac is described below
commit 4926a55b6ac839f71f3e5149d3ec962e914d9a0c
Author: HappenLee <[email protected]>
AuthorDate: Sat Oct 10 09:26:41 2026 +0800
[fix](be) Fix null-safe string joins in 4.1 (backport #65975) (#68809)
### What problem does this PR solve?
Issue Number: None
Related PR: #65975
Problem Summary:
Backport #65975 to `branch-4.1`.
A single-column string hash join using `<=>` can fail to match two NULL
keys when expression evaluation leaves different bytes in their nested
string columns. For example, joining `CONCAT(COALESCE(p.ch, ''),
CAST(p.v % 5 AS STRING)) <=> c.s` should match a NULL expression result
with a NULL `c.s`, but previously could miss that row depending on block
composition.
Normalize NULL keys to an empty `StringRef` in the serialization shared
by build and probe. Real empty strings remain in ordinary hash buckets,
separate from the dedicated NULL bucket. Also avoid passing default
empty refs to `memcmp`, and reject unsupported `<=>` ON predicates in
ASOF JOIN.
Resolve the parser conflict by retaining the 4.1 UNNEST handling and
changing only the ASOF equality check. Resolve the BE test conflict by
adding the original NULL normalization test without importing unrelated
master-only iterator tests. Adapt the existing ASOF regression to its
`nereids_p0` location on 4.1.
### Release note
Fix incorrect results of single-column null-safe string joins when NULL
keys come from expressions. ASOF JOIN rejects unsupported null-safe
equality ON predicates.
### Check List (For Author)
- Test:
- BE formatting: repository clang-format/check-format scripts passed
with clang-format 16.0.6.
- Regression: preserve the upstream expression-NULL, mixed-block,
NULL-versus-empty-string, and ASOF rejection suites. The generated
expected output is copied unchanged from the original commit; local
cluster execution has not been run.
- FE style: `mvn checkstyle:check -pl fe-core` passed (0 violations).
- FE Unit Test:
`NereidsParserTest#testParseAsofJoinRejectNullSafeEquality+testParseJoin`
passed locally (2 tests, 0 failures/errors).
- BE Unit Test: the original normalization test is preserved. Local ASAN
UT execution is blocked before compilation because the prebuilt
dependency bundle lacks `libpaimon_c.a`; no BE UT execution success is
claimed. The missing simdutf library was built with the repository
script before retrying.
- CI: `run buildall` requested for this backport.
- Behavior changed: Yes, correct NULL string key matching and reject
unsupported ASOF ON predicates.
- Does this need documentation: No.
---
be/src/core/string_ref.h | 4 +-
be/src/exec/common/hash_table/hash_map_context.h | 19 ++-
be/test/exec/hash_map/hash_table_method_test.cpp | 27 ++++
.../doris/nereids/parser/LogicalPlanBuilder.java | 3 +-
.../doris/nereids/parser/NereidsParserTest.java | 13 ++
.../join/test_null_safe_eq_join_string_key.out | 66 +++++++++
.../nereids_p0/join/asof/test_asof_join.groovy | 24 ++++
.../join/test_null_safe_eq_join_string_key.groovy | 154 +++++++++++++++++++++
8 files changed, 302 insertions(+), 8 deletions(-)
diff --git a/be/src/core/string_ref.h b/be/src/core/string_ref.h
index ca67ba91c06..c2424f98fe5 100644
--- a/be/src/core/string_ref.h
+++ b/be/src/core/string_ref.h
@@ -268,7 +268,9 @@ struct StringRef {
// ==
bool eq(const StringRef& other) const {
- return (size == other.size) && (memcmp(data, other.data, size) == 0);
+ // memcmp requires valid pointers even when size is 0, so
short-circuit empty strings
+ // to avoid passing nullptr data of default-constructed StringRef to
memcmp.
+ return (size == other.size) && (size == 0 || memcmp(data, other.data,
size) == 0);
}
bool operator==(const StringRef& other) const { return eq(other); }
diff --git a/be/src/exec/common/hash_table/hash_map_context.h
b/be/src/exec/common/hash_table/hash_map_context.h
index 19aff643c40..afb7ae3fc9d 100644
--- a/be/src/exec/common/hash_table/hash_map_context.h
+++ b/be/src/exec/common/hash_table/hash_map_context.h
@@ -371,19 +371,28 @@ struct MethodStringNoCache : public MethodBase<TData> {
}
void init_serialized_keys_impl(const ColumnRawPtrs& key_columns, uint32_t
num_rows,
- DorisVector<StringRef>& stored_keys) {
+ DorisVector<StringRef>& stored_keys, const
uint8_t* null_map) {
const IColumn& column = *key_columns[0];
const auto& nested_column =
column.is_nullable()
? assert_cast<const
ColumnNullable&>(column).get_nested_column()
: column;
- auto serialized_str = [](const auto& column_string,
DorisVector<StringRef>& stored_keys) {
+ // For join, rows with null keys are routed to a dedicated null bucket
and matched by
+ // raw key comparison, so their keys must be normalized to a canonical
empty StringRef
+ // to make null keys equal (e.g. single-column null-safe equal join).
The nested
+ // column of a null row may hold residual bytes left by expression
evaluation.
+ // Real empty strings are not affected: they are not null, so they are
still hashed
+ // into normal buckets and never meet the null keys in the null bucket.
+ auto serialized_str = [null_map](const auto& column_string,
+ DorisVector<StringRef>& stored_keys) {
const auto& offsets = column_string.get_offsets();
const auto* chars = column_string.get_chars().data();
stored_keys.resize(column_string.size());
for (size_t row = 0; row < column_string.size(); row++) {
- stored_keys[row] =
- StringRef(chars + offsets[row - 1], offsets[row] -
offsets[row - 1]);
+ stored_keys[row] = (null_map != nullptr && null_map[row])
+ ? StringRef()
+ : StringRef(chars + offsets[row -
1],
+ offsets[row] -
offsets[row - 1]);
}
};
if (nested_column.is_column_string64()) {
@@ -400,7 +409,7 @@ struct MethodStringNoCache : public MethodBase<TData> {
const uint8_t* null_map = nullptr, bool is_join
= false,
bool is_build = false, uint32_t bucket_size = 0)
override {
init_serialized_keys_impl(key_columns, num_rows,
- is_build ? _build_stored_keys :
_stored_keys);
+ is_build ? _build_stored_keys :
_stored_keys, null_map);
if (is_join) {
Base::init_join_bucket_num(num_rows, bucket_size, null_map);
} else {
diff --git a/be/test/exec/hash_map/hash_table_method_test.cpp
b/be/test/exec/hash_map/hash_table_method_test.cpp
index 75543290827..e556e2669a5 100644
--- a/be/test/exec/hash_map/hash_table_method_test.cpp
+++ b/be/test/exec/hash_map/hash_table_method_test.cpp
@@ -131,6 +131,33 @@ TEST(HashTableMethodTest, testMethodStringNoCache) {
{0, 1, -1, 3, -1, 4});
}
+// For join, null keys are routed to a dedicated null bucket and matched by
raw key
+// comparison. The nested column of a null row may hold residual bytes left by
expression
+// evaluation, so init_serialized_keys must normalize null keys to a canonical
empty
+// StringRef to make null keys equal (e.g. single-column null-safe equal join).
+TEST(HashTableMethodTest, testMethodStringNoCacheNullKeyNormalized) {
+ MethodStringNoCache<StringHashMap<IColumn::ColumnIndex>> method;
+
+ // Row 1 is null but its nested data holds residual bytes, row 2 is a real
empty string.
+ auto column =
+ ColumnHelper::create_nullable_column<DataTypeString>({"a",
"residual", ""}, {0, 1, 0});
+ ColumnRawPtrs key_columns {column.get()};
+ const auto& null_map = assert_cast<const
ColumnNullable&>(*column).get_null_map_data();
+
+ const uint32_t bucket_size = 8;
+ method.init_serialized_keys(key_columns, 3, null_map.data(), true, false,
bucket_size);
+
+ // Null key is normalized to a canonical empty StringRef instead of the
residual bytes.
+ EXPECT_TRUE(method._stored_keys[1] == StringRef());
+ // Non-null rows keep their real bytes, including the real empty string.
+ EXPECT_TRUE(method._stored_keys[0] == StringRef("a", 1));
+ EXPECT_TRUE(method._stored_keys[2] == StringRef("", 0));
+ // Null row is routed to the dedicated null bucket, real rows to normal
hash buckets.
+ EXPECT_EQ(method.bucket_nums[1], bucket_size);
+ EXPECT_LT(method.bucket_nums[0], bucket_size);
+ EXPECT_LT(method.bucket_nums[2], bucket_size);
+}
+
static AggregateDataPtr make_mapped(size_t val) {
return reinterpret_cast<AggregateDataPtr>(val);
}
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java
index 112d03b6dd7..61e4dfffc7b 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java
@@ -547,7 +547,6 @@ import org.apache.doris.nereids.trees.expressions.Default;
import org.apache.doris.nereids.trees.expressions.DefaultValueSlot;
import org.apache.doris.nereids.trees.expressions.DereferenceExpression;
import org.apache.doris.nereids.trees.expressions.Divide;
-import org.apache.doris.nereids.trees.expressions.EqualPredicate;
import org.apache.doris.nereids.trees.expressions.EqualTo;
import org.apache.doris.nereids.trees.expressions.Exists;
import org.apache.doris.nereids.trees.expressions.Expression;
@@ -4736,7 +4735,7 @@ public class LogicalPlanBuilder extends
DorisParserBaseVisitor<Object> {
}
List<Expression> conjuncts =
ExpressionUtils.extractConjunction(condition.get());
for (Expression expression : conjuncts) {
- if (!(expression instanceof EqualPredicate)) {
+ if (!(expression instanceof EqualTo)) {
throw new ParseException("ASOF JOIN's ON clause
must be one or more EQUAL(=) conjuncts",
join);
}
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/nereids/parser/NereidsParserTest.java
b/fe/fe-core/src/test/java/org/apache/doris/nereids/parser/NereidsParserTest.java
index 9fb470b46ca..beb74de7c1f 100644
---
a/fe/fe-core/src/test/java/org/apache/doris/nereids/parser/NereidsParserTest.java
+++
b/fe/fe-core/src/test/java/org/apache/doris/nereids/parser/NereidsParserTest.java
@@ -421,6 +421,19 @@ public class NereidsParserTest extends ParserTestBase {
Assertions.assertEquals(JoinType.CROSS_JOIN,
logicalJoin.getJoinType());
}
+ @Test
+ public void testParseAsofJoinRejectNullSafeEquality() {
+ parsePlan("SELECT t1.a FROM t1 ASOF INNER JOIN t2 "
+ + "MATCH_CONDITION(t1.dt < t2.dt) ON t1.id <=> t2.id")
+ .assertThrowsExactly(ParseException.class)
+ .assertMessageContains("ASOF JOIN's ON clause must be one or
more EQUAL(=) conjuncts");
+
+ parsePlan("SELECT t1.a FROM t1 ASOF LEFT JOIN t2 "
+ + "MATCH_CONDITION(t1.dt < t2.dt) ON t1.id <=> t2.id")
+ .assertThrowsExactly(ParseException.class)
+ .assertMessageContains("ASOF JOIN's ON clause must be one or
more EQUAL(=) conjuncts");
+ }
+
@Test
void parseJoinEmptyConditionError() {
parsePlan("select * from t1 LEFT JOIN t2")
diff --git
a/regression-test/data/query_p0/join/test_null_safe_eq_join_string_key.out
b/regression-test/data/query_p0/join/test_null_safe_eq_join_string_key.out
new file mode 100644
index 00000000000..43eed813d86
--- /dev/null
+++ b/regression-test/data/query_p0/join/test_null_safe_eq_join_string_key.out
@@ -0,0 +1,66 @@
+-- This file is automatically generated. You should know what you did if you
want to edit this
+-- !minimal_left_join --
+1 2
+2 1
+
+-- !minimal_inner_join --
+1 2
+2 1
+
+-- !minimal_oracle --
+1 2
+2 1
+
+-- !mixed_inner_join --
+0 0
+1 3
+1 4
+2 1
+3 3
+3 4
+4 2
+5 3
+5 4
+7 3
+7 4
+8 5
+9 3
+9 4
+
+-- !mixed_left_join --
+0 0
+1 3
+1 4
+2 1
+3 3
+3 4
+4 2
+5 3
+5 4
+6 \N
+7 3
+7 4
+8 5
+9 3
+9 4
+
+-- !mixed_oracle --
+0 0
+1 3
+1 4
+2 1
+3 3
+3 4
+4 2
+5 3
+5 4
+7 3
+7 4
+8 5
+9 3
+9 4
+
+-- !empty_string_inner_join --
+1 2
+2 1
+
diff --git a/regression-test/suites/nereids_p0/join/asof/test_asof_join.groovy
b/regression-test/suites/nereids_p0/join/asof/test_asof_join.groovy
index 6efc63dbbb6..611ca15a779 100644
--- a/regression-test/suites/nereids_p0/join/asof/test_asof_join.groovy
+++ b/regression-test/suites/nereids_p0/join/asof/test_asof_join.groovy
@@ -1247,6 +1247,30 @@ suite("test_asof_join", "nereids_p0") {
exception "ASOF JOIN's ON clause must be one or more EQUAL(=)
conjuncts"
}
+ test {
+ sql """
+ SELECT l.id, l.ts, r.id as rid, r.ts as rts, r.value
+ FROM asof_precision_left l
+ ASOF INNER JOIN asof_precision_right r
+ MATCH_CONDITION(l.ts >= r.ts)
+ ON l.grp <=> r.grp
+ ORDER BY l.id
+ """
+ exception "ASOF JOIN's ON clause must be one or more EQUAL(=)
conjuncts"
+ }
+
+ test {
+ sql """
+ SELECT l.id, l.ts, r.id as rid, r.ts as rts, r.value
+ FROM asof_precision_left l
+ ASOF LEFT JOIN asof_precision_right r
+ MATCH_CONDITION(l.ts >= r.ts)
+ ON l.grp <=> r.grp
+ ORDER BY l.id
+ """
+ exception "ASOF JOIN's ON clause must be one or more EQUAL(=)
conjuncts"
+ }
+
test {
sql """
SELECT l.id, l.ts, r.id as rid, r.ts as rts, r.value
diff --git
a/regression-test/suites/query_p0/join/test_null_safe_eq_join_string_key.groovy
b/regression-test/suites/query_p0/join/test_null_safe_eq_join_string_key.groovy
new file mode 100644
index 00000000000..7a87b2b0de7
--- /dev/null
+++
b/regression-test/suites/query_p0/join/test_null_safe_eq_join_string_key.groovy
@@ -0,0 +1,154 @@
+// 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.
+
+// Test single-column null-safe equal ( <=> ) hash join on string keys.
+// Null keys produced by expressions must match each other (NULL <=> NULL is
true),
+// and must never match a real empty string key.
+suite("test_null_safe_eq_join_string_key") {
+ sql """ DROP TABLE IF EXISTS test_nsej_string_p """
+ sql """ DROP TABLE IF EXISTS test_nsej_string_c """
+
+ sql """
+ CREATE TABLE test_nsej_string_p (
+ pk int,
+ ch char(10) not null,
+ v tinyint null
+ ) duplicate key(pk)
+ distributed by hash(pk) buckets 1
+ properties("replication_num" = "1");
+ """
+ sql """ insert into test_nsej_string_p values (1,'x',3),(2,'x',NULL) """
+
+ sql """
+ CREATE TABLE test_nsej_string_c (
+ pk int,
+ s varchar(100) null
+ ) duplicate key(pk)
+ distributed by hash(pk) buckets 1
+ properties("replication_num" = "1");
+ """
+ sql """ insert into test_nsej_string_c values (1, NULL), (2, 'x3') """
+
+ // Minimal case: NULL key comes from an expression, NULL <=> NULL must
match, so (2,1) is expected.
+ order_qt_minimal_left_join """
+ SELECT p.pk, c.pk AS cpk
+ FROM test_nsej_string_p p LEFT JOIN test_nsej_string_c c
+ ON CONCAT(COALESCE(p.ch,''), CAST((p.v % 5) AS STRING)) <=> c.s
+ ORDER BY 1, 2;
+ """
+
+ order_qt_minimal_inner_join """
+ SELECT p.pk, c.pk AS cpk
+ FROM test_nsej_string_p p INNER JOIN test_nsej_string_c c
+ ON CONCAT(COALESCE(p.ch,''), CAST((p.v % 5) AS STRING)) <=> c.s
+ ORDER BY 1, 2;
+ """
+
+ // The null-safe equal join must return the same result as its semantic
oracle.
+ order_qt_minimal_oracle """
+ SELECT p.pk, c.pk AS cpk
+ FROM test_nsej_string_p p LEFT JOIN test_nsej_string_c c
+ ON CONCAT(COALESCE(p.ch,''), CAST((p.v % 5) AS STRING)) = c.s
+ OR (CONCAT(COALESCE(p.ch,''), CAST((p.v % 5) AS STRING)) IS NULL
AND c.s IS NULL)
+ ORDER BY 1, 2;
+ """
+
+ sql """ DROP TABLE IF EXISTS test_nsej_string_a """
+ sql """ DROP TABLE IF EXISTS test_nsej_string_b """
+
+ sql """
+ CREATE TABLE test_nsej_string_a (
+ pk int,
+ v int null
+ ) duplicate key(pk)
+ distributed by hash(pk) buckets 3
+ properties("replication_num" = "1");
+ """
+ sql """
+ insert into test_nsej_string_a values
+
(0,0),(1,NULL),(2,2),(3,NULL),(4,4),(5,NULL),(6,6),(7,NULL),(8,8),(9,NULL)
+ """
+
+ sql """
+ CREATE TABLE test_nsej_string_b (
+ pk int,
+ s string null
+ ) duplicate key(pk)
+ distributed by hash(pk) buckets 3
+ properties("replication_num" = "1");
+ """
+ sql """
+ insert into test_nsej_string_b values
+ (0,'0'),(1,'2'),(2,'4'),(3,NULL),(4,NULL),(5,'8'),(6,'10')
+ """
+
+ // Mixed blocks: NULL keys from expression may hold residual bytes of
evaluated nested values,
+ // they must still match NULL keys on the other side.
+ order_qt_mixed_inner_join """
+ SELECT a.pk, b.pk AS bpk
+ FROM test_nsej_string_a a INNER JOIN test_nsej_string_b b
+ ON CAST(a.v AS STRING) <=> b.s
+ ORDER BY 1, 2;
+ """
+
+ order_qt_mixed_left_join """
+ SELECT a.pk, b.pk AS bpk
+ FROM test_nsej_string_a a LEFT JOIN test_nsej_string_b b
+ ON CAST(a.v AS STRING) <=> b.s
+ ORDER BY 1, 2;
+ """
+
+ order_qt_mixed_oracle """
+ SELECT a.pk, b.pk AS bpk
+ FROM test_nsej_string_a a INNER JOIN test_nsej_string_b b
+ ON CAST(a.v AS STRING) = b.s OR (CAST(a.v AS STRING) IS NULL AND b.s
IS NULL)
+ ORDER BY 1, 2;
+ """
+
+ // A normalized null key must not collide with a real empty string key.
+ sql """ DROP TABLE IF EXISTS test_nsej_string_e """
+ sql """ DROP TABLE IF EXISTS test_nsej_string_f """
+
+ sql """
+ CREATE TABLE test_nsej_string_e (
+ pk int,
+ v int null
+ ) duplicate key(pk)
+ distributed by hash(pk) buckets 1
+ properties("replication_num" = "1");
+ """
+ sql """ insert into test_nsej_string_e values (1,10),(2,NULL) """
+
+ sql """
+ CREATE TABLE test_nsej_string_f (
+ pk int,
+ s string null
+ ) duplicate key(pk)
+ distributed by hash(pk) buckets 1
+ properties("replication_num" = "1");
+ """
+ sql """ insert into test_nsej_string_f values (1,NULL),(2,'') """
+
+ // Row (1,10) yields a real empty string key, row (2,NULL) yields a null
key.
+ // Expected matches: (1,2) by empty string, (2,1) by NULL <=> NULL.
+ order_qt_empty_string_inner_join """
+ SELECT e.pk, f.pk AS fpk
+ FROM test_nsej_string_e e INNER JOIN test_nsej_string_f f
+ ON IF(e.v IS NOT NULL, '', NULL) <=> f.s
+ ORDER BY 1, 2;
+ """
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
