Uh oh!
There was an error while loading. Please reload this page.
[fix](be) Backport nullable hash state exception safety to branch-4.1 - #66517
Merged
Conversation
hello-stephen
commented
Aug 6, 2026
Contributor
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
HappenLee
commented
Aug 6, 2026
ContributorAuthor
run buildall |
yiguoleiforce-pushed
the
backport-pr-66102-4.1
branch
from
August 6, 2026 09:26
becbc9a to
a7e4439Compareyiguolei
commented
Aug 6, 2026
Contributor
run buildall |
Issue Number: None Related PR: None Problem Summary: Nullable aggregation hash tables marked the null-key entry as present before its aggregate state creator completed. If allocation or aggregate-state creation threw, cleanup treated the uninitialized null-key state as valid and could crash while destroying it. Publish the null-key flag only after successful creation, initialize its storage, and cover mapped, void, and StringHashMap batch paths with exception-safety tests. Fix a BE crash when nullable aggregation state creation fails under memory pressure. - Test: Unit Test added; ASAN_UT test object compiled successfully; clang-tidy and format checks passed - Behavior changed: Yes, failed null-key state creation is no longer published as a valid hash entry - Does this need documentation: No
Issue Number: None Related PR: apache#66102 Problem Summary: Nullable LIMIT aggregation creators construct resource-owning null-key aggregate state before updating the top-N limit columns or heap. If that post-construction work throws, delayed null-key publication leaves cleanup unaware of the live state, so its destructor is skipped. Add an explicit commit/rollback boundary that destroys a fully constructed state when post-create work fails and publishes the mapped pointer only after the work succeeds. Cover ordinary nullable and StringHashMap batch paths with post-construction failure and successful retry tests. Fix cleanup of nullable aggregation states when LIMIT maintenance fails after state construction. - Test: Unit Test - GLIBC_COMPATIBILITY=OFF ./run-be-ut.sh -j 48 --run --filter=HashTableMethodTest.* - build-support/check-format.sh - build-support/run-clang-tidy.sh --build-dir be/ut_build_ASAN - Behavior changed: Yes. Fully constructed nullable aggregation state is rolled back if post-construction LIMIT maintenance throws. - Does this need documentation: No
### What problem does this PR solve? Issue Number: None Related PR: apache#66102 Problem Summary: Keep the backported hash table exception-safety test aligned with the branch-4.1 C++ formatting rules after resolving the cherry-pick conflict. ### Release note None ### Check List (For Author) - Test: build-support/check-format.sh - Behavior changed: No - Does this need documentation: No
yiguoleiforce-pushed
the
backport-pr-66102-4.1
branch
from
August 7, 2026 01:16
a7e4439 to
4cccbe7Compareyiguolei
commented
Aug 7, 2026
Contributor
run buildall |
### What problem does this PR solve? Issue Number: None Related PR: apache#66102 Problem Summary: The branch-4.1 backport added exception-safety tests that use commit_aggregate_state, AggDataNullable, and AggregatedDataWithNullableShortStringKey, but the conflict resolution omitted the agg_utils.h include that declares them. Include the aggregation utilities directly so the hash table unit test compiles without relying on unavailable declarations. ### Release note None ### Check List (For Author) - Test: build-support/check-format.sh; git diff --check - Behavior changed: No - Does this need documentation: No
HappenLee
commented
Aug 7, 2026
ContributorAuthor
run buildall |
hello-stephen
commented
Aug 7, 2026
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
yiguolei
approved these changes
Aug 7, 2026
Uh oh!
There was an error while loading. Please reload this page.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What problem does this PR solve?
Issue Number: None
Related PR: #66102
Problem Summary:
Backport #66102 to branch-4.1. Nullable aggregation hash tables marked the null-key entry as present before aggregate-state creation completed. If creation or later LIMIT maintenance threw, cleanup could either destroy uninitialized storage or miss destruction of a live aggregate state.
This backport publishes the null-key entry only after successful creation, value-initializes null-key storage, and rolls back completed aggregate states when post-construction LIMIT maintenance fails. The conflict in
hash_table_method_test.cppwas resolved by carrying over only the exception-safety coverage from #66102; unrelated tests that exist only on master were not introduced into branch-4.1.Release note
Fix cleanup of nullable aggregation states when creation or post-construction LIMIT maintenance fails.
Check List (For Author)
PATH=/mnt/disk6/common/ldb_toolchain_toucan/bin:$PATH build-support/check-format.sh(passed with clang-format 16.0.6)HashTableMethodTest.*binary/link/run was not completed locally because another concurrent Doris UT build caused prolonged resource contention; PR CI is expected to complete the full validation.