github-actions[bot] commented on code in PR #61371:
URL: https://github.com/apache/doris/pull/61371#discussion_r2944339767
##########
be/src/runtime/runtime_profile_counter_tree_node.cpp:
##########
@@ -157,4 +157,16 @@ PProfileCounter RuntimeProfileCounterTreeNode::to_proto()
const {
return pcounter;
}
+void RuntimeProfileCounterTreeNode::pretty_print(std::ostream* s, const
std::string& prefix) const {
+ // Print this node's counter (skip ROOT_COUNTER which has no counter)
+ if (name != RuntimeProfile::ROOT_COUNTER && counter != nullptr) {
+ counter->pretty_print(s, prefix, name);
+ }
+
+ // Recursively print children with additional indentation
+ for (const auto& child : children) {
+ child.pretty_print(s, prefix + " ");
+ }
+}
+
} // namespace doris
Review Comment:
**Bug: Indentation regression in `pretty_print`**
The old code in `RuntimeProfile::print_child_counters()` (line 787-804)
printed ROOT_COUNTER's direct children at the `prefix` indentation level:
```cpp
// Old: print_child_counters(prefix, ROOT_COUNTER, ...)
for (child_counter : child_counters) {
counter->pretty_print(s, prefix, child_name); // <-- at 'prefix'
print_child_counters(prefix + " ", child_name, ...); // grandchildren
at 'prefix+2'
}
```
The new code prints ROOT_COUNTER's direct children at `prefix + " "` (one
extra indent level):
```cpp
// New: ROOT_COUNTER node skips printing itself, then:
for (child : children) {
child.pretty_print(s, prefix + " "); // <-- already adds indent here
// Inside child.pretty_print: prints counter at prefix + " " (wrong!)
}
```
This means top-level counters that were previously at `prefix` indentation
are now shifted right by 2 spaces. To fix, the ROOT_COUNTER node should pass
`prefix` (not `prefix + " "`) to its direct children, or alternatively, the
initial call in `RuntimeProfile::pretty_print` should call
`counter_tree.pretty_print(s, prefix)` and have `pretty_print` not add
indentation at the ROOT level. The simplest fix:
```cpp
void RuntimeProfileCounterTreeNode::pretty_print(std::ostream* s, const
std::string& prefix) const {
if (name != RuntimeProfile::ROOT_COUNTER && counter != nullptr) {
counter->pretty_print(s, prefix, name);
}
// For ROOT_COUNTER, don't add indentation to children
const std::string& child_prefix = (name == RuntimeProfile::ROOT_COUNTER)
? prefix : prefix + " ";
for (const auto& child : children) {
child.pretty_print(s, child_prefix);
}
}
```
##########
be/src/exec/operator/multi_cast_data_stream_sink.cpp:
##########
@@ -27,15 +27,14 @@ namespace doris {
std::string MultiCastDataStreamSinkLocalState::name_suffix() {
auto* parent = static_cast<MultiCastDataStreamSinkOperatorX*>(_parent);
auto& dest_ids = parent->dests_id();
- std::string result = "(";
+ std::string dest_list;
for (size_t i = 0; i < dest_ids.size(); ++i) {
if (i > 0) {
- result += ", ";
+ dest_list += ",";
}
- result += fmt::format("dest_id={}", dest_ids[i]);
+ dest_list += std::to_string(dest_ids[i]);
Review Comment:
**Nit: Missing `nereids_id` in multicast sink suffix**
All other operator `name_suffix()` implementations include `nereids_id` when
it is not -1 (e.g., `exchange_sink_operator.cpp`, `olap_scan_operator.h`,
generic `operator.cpp`). This implementation only includes `id` and `dest_ids`,
never `nereids_id`. Is this intentional? If multicast sinks always have
`nereids_id == -1`, this is fine. Otherwise, it should follow the same pattern:
```cpp
if (_parent->nereids_id() == -1) {
return fmt::format("(id={}, dest_ids=[{}])", parent->operator_id(),
dest_list);
} else {
return fmt::format("(nereids_id={}, id={}, dest_ids=[{}])",
_parent->nereids_id(), parent->operator_id(), dest_list);
}
```
--
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]
