This is an automated email from the ASF dual-hosted git repository.
HappenLee 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 e5b67e095b4 [fix](asof join) Fix wrong result of single-column
null-safe eq join on string keys (#65975)
e5b67e095b4 is described below
commit e5b67e095b4fd40acd656ad25aed6bf5d8b52718
Author: HappenLee <[email protected]>
AuthorDate: Thu Jul 30 09:46:33 2026 +0800
[fix](asof join) Fix wrong result of single-column null-safe eq join on
string keys (#65975)
Problem Summary:
For a single-column null-safe equal join (`<=>`) on a string key, NULL
keys produced by expressions may fail to match each other, and the
result can even change with the number of BEs or the tablet/bucket
distribution.
Minimal reproduction (2 tables, 4 rows, single BE, no aggregation):
create table mini_p (pk int, ch char(10) not null, v tinyint null)
duplicate key(pk) distributed by hash(pk) buckets 1
properties("replication_num"="1");
insert into mini_p values (1,'x',3),(2,'x',NULL);
create table mini_c (pk int, s varchar(100) null)
duplicate key(pk) distributed by hash(pk) buckets 1
properties("replication_num"="1");
insert into mini_c values (1, NULL), (2, 'x3');
SELECT p.pk, c.pk AS cpk FROM mini_p p LEFT JOIN mini_c c
ON CONCAT(COALESCE(p.ch,''), CAST((p.v % 5) AS STRING)) <=> c.s ORDER BY
1, 2;
-- wrong: (1,2),(2,NULL); correct: (1,2),(2,1), because NULL <=> NULL is
true
Root cause: for a single-column string join key, the hash join splits
the nullable key column into the nested column plus an external null map
(`_serialize_null_into_key=false`), and routes null rows to a dedicated
null bucket (`init_join_bucket_num`) where they are matched by raw key
bytes (`_eq`). The design requires both sides to first normalize the
nested data of null rows to the default value via
`replace_column_null_data`, but `ColumnString`/`ColumnString64` do not
implement it (only `ColumnVector`/`ColumnDecimal` do), so the
normalization is a silent no-op.
`MethodStringNoCache::init_serialized_keys_impl` then uses the residual
bytes left by expression evaluation in the nested column as the key of a
null row, so `NULL <=> NULL` matches only when the residual bytes on
both sides happen to be equal (e.g. an all-NULL block short-circuits to
the default value, while a mixed block leaves evaluated residue). That
is why the result depends on block composition, which in turn depends on
BE count, tablet distribution and parallelism. Numeric and decimal keys
are not affected because their columns implement
`replace_column_null_data`; multi-column keys are not affected because
the null flag is serialized into the key.
Fix: in `MethodStringNoCache::init_serialized_keys_impl`, when a null
map is provided, normalize the stored key of null rows to a canonical
empty `StringRef`. Build and probe go through the same function so null
keys on both sides are naturally symmetric. 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 dedicated null bucket.
### Release note
Fix wrong results of single-column null-safe equal join (`<=>`) on
string keys when the NULL key comes from an expression.
### Check List (For Author)
- Test:
- Regression test: new suite
`query_p0/join/test_null_safe_eq_join_string_key` covering minimal
reproduction, mixed-block cases compared with the `= OR (both IS NULL)`
oracle, and NULL-vs-empty-string collision cases. Passed on a local
cluster.
- Unit Test: new case
`HashTableMethodTest.testMethodStringNoCacheNullKeyNormalized`
(compiled; the local ASAN UT binary crashes at process init due to a
pre-existing libomp/openblas issue unrelated to this change, so it could
not be executed locally).
- Behavior changed: Yes - `NULL <=> NULL` on single-column string hash
join keys now matches correctly; previous results could be wrong or
unstable across different BE counts/plan shapes.
- 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 +++++++++
.../query_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 159fe908ca8..cf9cbcf3a37 100644
--- a/be/src/core/string_ref.h
+++ b/be/src/core/string_ref.h
@@ -267,7 +267,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 9c5bbab2101..804d2e48020 100644
--- a/be/src/exec/common/hash_table/hash_map_context.h
+++ b/be/src/exec/common/hash_table/hash_map_context.h
@@ -375,19 +375,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 =
is_column_nullable(column)
? 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()) {
@@ -404,7 +413,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 697ce283751..de2f0a7ac63 100644
--- a/be/test/exec/hash_map/hash_table_method_test.cpp
+++ b/be/test/exec/hash_map/hash_table_method_test.cpp
@@ -130,6 +130,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);
+}
+
// Verify that iterating a DataWithNullKey hash map via
init_iterator()/begin/end
// does NOT visit the null key entry. The null key must be accessed separately
// through has_null_key_data()/get_null_key_data().
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 51dacb59e83..b83ee578038 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
@@ -539,7 +539,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;
@@ -4739,7 +4738,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 1bd7dda5c33..c0d6b951953 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
@@ -487,6 +487,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/query_p0/join/asof/test_asof_join.groovy
b/regression-test/suites/query_p0/join/asof/test_asof_join.groovy
index 66ed0b88f60..1667f5f47f6 100644
--- a/regression-test/suites/query_p0/join/asof/test_asof_join.groovy
+++ b/regression-test/suites/query_p0/join/asof/test_asof_join.groovy
@@ -1247,6 +1247,30 @@ suite("test_asof_join", "query_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]
