github-actions[bot] commented on code in PR #65635:
URL: https://github.com/apache/doris/pull/65635#discussion_r3585464705
##########
be/src/exec/common/hash_table/hash_table_set_build.h:
##########
@@ -41,11 +43,18 @@ struct HashTableBuild {
};
auto creator_for_null_key = [&](auto& mapped) { mapped = {k}; };
- for (; k < _rows; ++k) {
- if (k % CHECK_FRECUENCY == 0) {
- RETURN_IF_CANCELLED(_state);
+ // The batch path regresses StringRef keys, including serialized and
nullable strings.
+ if constexpr (std::is_same_v<typename HashTableContext::Key,
StringRef>) {
+ for (; k < _rows; ++k) {
+ if (k % CHECK_FRECUENCY == 0) {
+ RETURN_IF_CANCELLED(_state);
+ }
+ hash_table_ctx.lazy_emplace(key_getter, k, creator,
creator_for_null_key);
}
- hash_table_ctx.lazy_emplace(key_getter, k, creator,
creator_for_null_key);
+ } else {
+ RETURN_IF_CANCELLED(_state);
Review Comment:
Here we only check cancellation once before entering
`lazy_emplace_batch_void`, but for non-string contexts that helper is a plain
loop over `num_rows` with no cancellation hook. Because `SetSinkOperatorX`
merges all build-side blocks until EOS before calling `_process_build_block`,
`_rows` can be the whole left input, only bounded by `uint32_t::max()`, so a
cancelled query can continue inserting numeric/fixed keys for a long time.
Please keep the batch path cancellation-aware, for example by having the helper
return `Status` or by chunking it so `RETURN_IF_CANCELLED` is evaluated every
`CHECK_FRECUENCY` rows.
--
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]
