Skip to content

[fix](be) Fix wrong result of single-column null-safe eq join on string keys#65975

Open
HappenLee wants to merge 1 commit into
apache:masterfrom
HappenLee:fix-null-safe-join-string-key
Open

[fix](be) Fix wrong result of single-column null-safe eq join on string keys#65975
HappenLee wants to merge 1 commit into
apache:masterfrom
HappenLee:fix-null-safe-join-string-key

Conversation

@HappenLee

Copy link
Copy Markdown
Contributor

What problem does this PR solve?

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

…ng keys

### What problem does this PR solve?

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
@hello-stephen

Copy link
Copy Markdown
Contributor

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

  1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
  2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
  3. What features were added. Why was this function added?
  4. Which code was refactored and why was this part of the code refactored?
  5. Which functions were optimized and what is the difference before and after the optimization?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants