executor, util: fix hash join v2 null build key
Project: pingcap/tidb | Language: Go | Concepts: algorithmic-optimization, data-structures, testing-strategy | Difficulty: mid
Link: https://github.com/pingcap/tidb/pull/70786
The problem
Hash Join V2 was incorrectly treating a TypeNull (i.e., SQL NULL) build key as a valid serialized key. This meant that when a probe key was an empty binary string, the join could mistakenly consider the NULL build key as matching, leading to incorrect filtering in anti semi joins. Specifically, a NULL row could be filtered out when it should not be, violating SQL semantics for NOT IN and anti joins involving NULLs.
Why it's non-trivial
A naive fix might only check for NULLs during join comparison, but the bug occurs earlier—during key serialization and buffer allocation. If NULLs are not excluded at the key-preparation stage, empty binary keys and NULL keys can be treated as the same, causing subtle correctness issues in the join algorithm. Proper handling requires marking NULL rows so they are never considered as valid join keys in the hash table.
The approach
1. Marking NULL rows during serialized key buffer pre-allocation
In the function responsible for pre-allocating buffers for serialized join keys, the code now explicitly marks rows with TypeNull as skippable, so they are not included as valid join keys:
@@ -619,6 +619,9 @@ func preAllocForSerializedKeyBuffer(
case mysql.TypeNull:
+ for _, physicalRowIndex := range usedRows {
+ canSkip(physicalRowIndex)
+ }
What this does:
For columns of type TypeNull, every row index in usedRows is marked via canSkip, ensuring these rows are excluded from the set of valid join keys. This prevents empty binary keys from colliding with NULL keys in the hash join.
2. Documenting future work for NULL-safe joins
A comment was added in the serialization implementation to note that if NULL-safe equal joins are ever needed, they will require serializing TypeNull as a valid join key:
@@ -836,6 +839,7 @@ func serializeKeysImpl(
case mysql.TypeNull:
+ // TODO: NULL-safe equal joins need to serialize TypeNull as a valid join key.
What this does:
This comment reminds future maintainers that the current logic is not sufficient for NULL-safe joins (like <=> in MySQL), and that further work would be needed to handle those cases.
3. Adding regression and unit tests
A new unit test was added to directly verify that a NULL build key does not match an empty binary probe key in an anti-semi join:
+func TestAntiSemiJoinTypeNullBuildKey(t *testing.T) {
+ ...
+ leftChk := chunk.NewChunkWithCapacity([]*types.FieldType{nullTp}, 1)
+ leftChk.AppendNull(0)
+ rightChk := chunk.NewChunkWithCapacity([]*types.FieldType{blobTp}, 1)
+ rightChk.AppendBytes(0, []byte{})
+ ...
+ result := getSortedResults(t, buildHashJoinV2Exec(info), []*types.FieldType{nullTp})
+ require.Len(t, result, 1)
+ require.True(t, result[0].IsNull(0))
+}
What this does:
Builds a left table with a single NULL, a right table with a single empty binary, and verifies that after an anti-semi join, the result is still a single NULL row—demonstrating correct handling.
Integration tests were also added:
+# https://github.com/pingcap/tidb/issues/70672
+set @@tidb_hash_join_version = optimized;
+drop table if exists t0;
+create table t0(c0 BLOB(249));
+insert into t0 values ('');
+select v.c0
+from (select NULL as c0 from t0) v
+where v.c0 NOT IN (
+ select t0.c0
+ from t0
+ where t0.c0 = v.c0
+);
+drop table t0;
+set @@tidb_hash_join_version = default;
What this does:
Runs a real SQL query reproducing the bug scenario, ensuring that the result is correct (NULL is not filtered out).
From the review thread
No notable review discussion on this PR.
Takeaways
- Always handle NULLs explicitly in join algorithms—treating them as valid keys can break SQL semantics.
- Fixes for subtle bugs should be accompanied by both unit and integration regression tests to prevent recurrence.
- When serializing or hashing keys for data structures, ensure special values (like NULL) cannot collide with valid data (like empty strings or zero values).