github-actions[bot] commented on code in PR #66102:
URL: https://github.com/apache/doris/pull/66102#discussion_r3664474060
##########
be/src/exec/common/columns_hashing.h:
##########
@@ -128,18 +128,17 @@ struct HashMethodSingleLowNullableColumn : public
SingleColumnMethod {
size_t hash_value, Func&& f,
CreatorForNull&& null_creator) {
if (key_column->is_null_at(row)) {
- bool has_null_key = data.has_null_key_data();
- data.has_null_key_data() = true;
-
if constexpr (std::is_same_v<Mapped, void>) {
- if (!has_null_key) {
+ if (!data.has_null_key_data()) {
std::forward<CreatorForNull>(null_creator)();
+ data.has_null_key_data() = true;
}
return nullptr;
} else {
- if (!has_null_key) {
+ if (!data.has_null_key_data()) {
std::forward<CreatorForNull>(null_creator)(
data.template get_null_key_data<Mapped>());
+ data.has_null_key_data() = true;
Review Comment:
[P1] Preserve cleanup after successful null-state construction
This assignment is skipped whenever the entire null creator throws, not only
when aggregate-state creation itself fails. Both nullable LIMIT creators
successfully call `_create_agg_status(mapped)` and then call
`refresh_top_limit` / `_refresh_limit_heap`; those column insertions and heap
emplacements can still allocate and throw. On that path this flag (and the
mirrored StringHashMap flag) stays false, while close/reset destroy the null
state only when the flag is true, so a fully constructed resource-owning
aggregate state is never destroyed. Please add an explicit construction
commit/rollback boundary—perform later throwing work before construction where
valid, publish immediately after successful construction, or destroy the
constructed state before rethrowing—and cover a creator that constructs/assigns
the mapped state before throwing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
