github-actions[bot] commented on code in PR #17187:
URL: https://github.com/apache/doris/pull/17187#discussion_r1118393210
##########
be/src/runtime/runtime_state.h:
##########
@@ -195,6 +196,10 @@ class RuntimeState {
int64_t load_job_id() const { return _load_job_id; }
+ void set_shared_scan_opt(bool shared_scan_opt) { _shared_scan_opt =
shared_scan_opt; }
+
+ bool shared_scan_opt() { return _shared_scan_opt; }
Review Comment:
warning: method 'shared_scan_opt' can be made const
[readability-make-member-function-const]
```suggestion
bool shared_scan_opt() const { return _shared_scan_opt; }
```
##########
be/src/vec/exec/scan/pip_scanner_context.h:
##########
@@ -33,20 +33,74 @@ class PipScannerContext : public vectorized::ScannerContext
{
: vectorized::ScannerContext(state, parent, input_tuple_desc,
output_tuple_desc,
scanners, limit,
max_bytes_in_blocks_queue) {}
- void _update_block_queue_empty() override { _blocks_queue_empty =
_blocks_queue.empty(); }
+ void _update_block_queue_empty(int id) override {
+ _blocks_queue_empty[id] = _blocks_queues[id].empty();
+ }
+
+ Status get_block_from_queue(RuntimeState* state, vectorized::BlockUPtr*
block, bool* eos,
+ int id, bool wait = false) override {
+ {
+ std::unique_lock<std::mutex> l(_transfer_lock);
+ if (state->is_cancelled()) {
Review Comment:
warning: member access into incomplete type 'doris::RuntimeState'
[clang-diagnostic-error]
```cpp
if (state->is_cancelled()) {
^
```
**be/src/runtime/memory/mem_tracker_limiter.h:31:** forward declaration of
'doris::RuntimeState'
```cpp
class RuntimeState;
^
```
##########
be/src/vec/exec/scan/vscan_node.cpp:
##########
@@ -117,13 +134,25 @@ Status VScanNode::alloc_resource(RuntimeState* state) {
return Status::OK();
}
- std::list<VScanner*> scanners;
- RETURN_IF_ERROR(_init_scanners(&scanners));
- if (scanners.empty()) {
- _eos = true;
+ if (_is_pipeline_scan) {
+ if (_should_create_scanner) {
+ auto status = _prepare_scanners();
+ if (_shared_scan_opt) {
+ _shared_scanner_controller->set_scanner_context(
+ id(), _num_scanners->value() == 0 ? nullptr :
_scanner_ctx);
+ RETURN_IF_ERROR(status);
+ if (!_eos)
_scanner_ctx->set_max_queue_size(state->query_parallel_instance_num());
Review Comment:
warning: statement should be inside braces
[readability-braces-around-statements]
```suggestion
if (!_eos) {
_scanner_ctx->set_max_queue_size(state->query_parallel_instance_num());
}
```
##########
be/src/vec/functions/function.cpp:
##########
@@ -37,48 +37,53 @@ namespace doris::vectorized {
ColumnPtr wrap_in_nullable(const ColumnPtr& src, const Block& block, const
ColumnNumbers& args,
size_t result, size_t input_rows_count) {
ColumnPtr result_null_map_column;
-
/// If result is already nullable.
ColumnPtr src_not_nullable = src;
+ MutableColumnPtr mutable_result_null_map_column;
- if (src->only_null())
- return src;
- else if (auto* nullable = check_and_get_column<ColumnNullable>(*src)) {
+ if (auto* nullable = check_and_get_column<ColumnNullable>(*src)) {
src_not_nullable = nullable->get_nested_column_ptr();
result_null_map_column = nullable->get_null_map_column_ptr();
}
for (const auto& arg : args) {
const ColumnWithTypeAndName& elem = block.get_by_position(arg);
- if (!elem.type->is_nullable()) continue;
+ if (!elem.column->is_nullable()) continue;
Review Comment:
warning: statement should be inside braces
[readability-braces-around-statements]
```suggestion
if (!elem.column->is_nullable()) { continue;
}
```
##########
be/src/vec/functions/function.cpp:
##########
@@ -37,48 +37,53 @@
ColumnPtr wrap_in_nullable(const ColumnPtr& src, const Block& block, const
ColumnNumbers& args,
size_t result, size_t input_rows_count) {
ColumnPtr result_null_map_column;
-
/// If result is already nullable.
ColumnPtr src_not_nullable = src;
+ MutableColumnPtr mutable_result_null_map_column;
- if (src->only_null())
- return src;
- else if (auto* nullable = check_and_get_column<ColumnNullable>(*src)) {
+ if (auto* nullable = check_and_get_column<ColumnNullable>(*src)) {
src_not_nullable = nullable->get_nested_column_ptr();
result_null_map_column = nullable->get_null_map_column_ptr();
}
for (const auto& arg : args) {
const ColumnWithTypeAndName& elem = block.get_by_position(arg);
- if (!elem.type->is_nullable()) continue;
+ if (!elem.column->is_nullable()) continue;
+ bool is_const = is_column_const(*elem.column);
/// Const Nullable that are NULL.
- if (elem.column->only_null())
+ if (is_const && assert_cast<const
ColumnConst*>(elem.column.get())->only_null())
return
block.get_by_position(result).type->create_column_const(input_rows_count,
Null());
+ if (is_const) continue;
- if (is_column_const(*elem.column)) continue;
-
- if (auto* nullable =
check_and_get_column<ColumnNullable>(*elem.column)) {
+ if (auto* nullable = assert_cast<const
ColumnNullable*>(elem.column.get())) {
const ColumnPtr& null_map_column =
nullable->get_null_map_column_ptr();
if (!result_null_map_column) {
- result_null_map_column =
null_map_column->clone_resized(null_map_column->size());
+ result_null_map_column =
null_map_column->clone_resized(input_rows_count);
} else {
- MutableColumnPtr mutable_result_null_map_column =
- (*std::move(result_null_map_column)).assume_mutable();
+ if (!mutable_result_null_map_column)
+ mutable_result_null_map_column =
+
(*std::move(result_null_map_column)).assume_mutable();
NullMap& result_null_map =
assert_cast<ColumnUInt8&>(*mutable_result_null_map_column).get_data();
const NullMap& src_null_map =
assert_cast<const
ColumnUInt8&>(*null_map_column).get_data();
VectorizedUtils::update_null_map(result_null_map,
src_null_map);
- result_null_map_column =
std::move(mutable_result_null_map_column);
}
}
}
- if (!result_null_map_column) return make_nullable(src);
+ if (!result_null_map_column) {
+ if (is_column_const(*src)) {
+ return ColumnConst::create(
+ make_nullable(assert_cast<const
ColumnConst&>(*src).get_data_column_ptr(), 0),
Review Comment:
warning: converting integer literal to bool, use bool literal instead
[modernize-use-bool-literals]
```suggestion
make_nullable(assert_cast<const
ColumnConst&>(*src).get_data_column_ptr(), false),
```
##########
be/src/vec/functions/function.cpp:
##########
@@ -37,48 +37,53 @@
ColumnPtr wrap_in_nullable(const ColumnPtr& src, const Block& block, const
ColumnNumbers& args,
size_t result, size_t input_rows_count) {
ColumnPtr result_null_map_column;
-
/// If result is already nullable.
ColumnPtr src_not_nullable = src;
+ MutableColumnPtr mutable_result_null_map_column;
- if (src->only_null())
- return src;
- else if (auto* nullable = check_and_get_column<ColumnNullable>(*src)) {
+ if (auto* nullable = check_and_get_column<ColumnNullable>(*src)) {
src_not_nullable = nullable->get_nested_column_ptr();
result_null_map_column = nullable->get_null_map_column_ptr();
}
for (const auto& arg : args) {
const ColumnWithTypeAndName& elem = block.get_by_position(arg);
- if (!elem.type->is_nullable()) continue;
+ if (!elem.column->is_nullable()) continue;
+ bool is_const = is_column_const(*elem.column);
/// Const Nullable that are NULL.
- if (elem.column->only_null())
+ if (is_const && assert_cast<const
ColumnConst*>(elem.column.get())->only_null())
return
block.get_by_position(result).type->create_column_const(input_rows_count,
Null());
+ if (is_const) continue;
- if (is_column_const(*elem.column)) continue;
-
- if (auto* nullable =
check_and_get_column<ColumnNullable>(*elem.column)) {
+ if (auto* nullable = assert_cast<const
ColumnNullable*>(elem.column.get())) {
const ColumnPtr& null_map_column =
nullable->get_null_map_column_ptr();
if (!result_null_map_column) {
- result_null_map_column =
null_map_column->clone_resized(null_map_column->size());
+ result_null_map_column =
null_map_column->clone_resized(input_rows_count);
} else {
- MutableColumnPtr mutable_result_null_map_column =
- (*std::move(result_null_map_column)).assume_mutable();
+ if (!mutable_result_null_map_column)
Review Comment:
warning: statement should be inside braces
[readability-braces-around-statements]
```suggestion
if (!mutable_result_null_map_column) {
```
be/src/vec/functions/function.cpp:66:
```diff
-
(*std::move(result_null_map_column)).assume_mutable();
+
(*std::move(result_null_map_column)).assume_mutable();
+ }
```
##########
be/src/vec/exec/scan/vscan_node.cpp:
##########
@@ -117,13 +134,25 @@
return Status::OK();
}
- std::list<VScanner*> scanners;
- RETURN_IF_ERROR(_init_scanners(&scanners));
- if (scanners.empty()) {
- _eos = true;
+ if (_is_pipeline_scan) {
+ if (_should_create_scanner) {
+ auto status = _prepare_scanners();
+ if (_shared_scan_opt) {
+ _shared_scanner_controller->set_scanner_context(
+ id(), _num_scanners->value() == 0 ? nullptr :
_scanner_ctx);
+ RETURN_IF_ERROR(status);
+ if (!_eos)
_scanner_ctx->set_max_queue_size(state->query_parallel_instance_num());
+ } else {
+ RETURN_IF_ERROR(status);
+ }
+ } else if (_shared_scanner_controller->scanner_context_is_ready(id()))
{
+ _scanner_ctx =
_shared_scanner_controller->get_scanner_context(id());
+ if (!_scanner_ctx) _eos = true;
Review Comment:
warning: statement should be inside braces
[readability-braces-around-statements]
```suggestion
if (!_scanner_ctx) { _eos = true;
}
```
##########
be/src/vec/functions/function.cpp:
##########
@@ -37,48 +37,53 @@
ColumnPtr wrap_in_nullable(const ColumnPtr& src, const Block& block, const
ColumnNumbers& args,
size_t result, size_t input_rows_count) {
ColumnPtr result_null_map_column;
-
/// If result is already nullable.
ColumnPtr src_not_nullable = src;
+ MutableColumnPtr mutable_result_null_map_column;
- if (src->only_null())
- return src;
- else if (auto* nullable = check_and_get_column<ColumnNullable>(*src)) {
+ if (auto* nullable = check_and_get_column<ColumnNullable>(*src)) {
src_not_nullable = nullable->get_nested_column_ptr();
result_null_map_column = nullable->get_null_map_column_ptr();
}
for (const auto& arg : args) {
const ColumnWithTypeAndName& elem = block.get_by_position(arg);
- if (!elem.type->is_nullable()) continue;
+ if (!elem.column->is_nullable()) continue;
+ bool is_const = is_column_const(*elem.column);
/// Const Nullable that are NULL.
- if (elem.column->only_null())
+ if (is_const && assert_cast<const
ColumnConst*>(elem.column.get())->only_null())
Review Comment:
warning: statement should be inside braces
[readability-braces-around-statements]
```suggestion
if (is_const && assert_cast<const
ColumnConst*>(elem.column.get())->only_null()) {
```
be/src/vec/functions/function.cpp:56:
```diff
-
Null());
+
Null());
+ }
```
##########
be/src/vec/functions/function.cpp:
##########
@@ -37,48 +37,53 @@
ColumnPtr wrap_in_nullable(const ColumnPtr& src, const Block& block, const
ColumnNumbers& args,
size_t result, size_t input_rows_count) {
ColumnPtr result_null_map_column;
-
/// If result is already nullable.
ColumnPtr src_not_nullable = src;
+ MutableColumnPtr mutable_result_null_map_column;
- if (src->only_null())
- return src;
- else if (auto* nullable = check_and_get_column<ColumnNullable>(*src)) {
+ if (auto* nullable = check_and_get_column<ColumnNullable>(*src)) {
src_not_nullable = nullable->get_nested_column_ptr();
result_null_map_column = nullable->get_null_map_column_ptr();
}
for (const auto& arg : args) {
const ColumnWithTypeAndName& elem = block.get_by_position(arg);
- if (!elem.type->is_nullable()) continue;
+ if (!elem.column->is_nullable()) continue;
+ bool is_const = is_column_const(*elem.column);
/// Const Nullable that are NULL.
- if (elem.column->only_null())
+ if (is_const && assert_cast<const
ColumnConst*>(elem.column.get())->only_null())
return
block.get_by_position(result).type->create_column_const(input_rows_count,
Null());
+ if (is_const) continue;
Review Comment:
warning: statement should be inside braces
[readability-braces-around-statements]
```suggestion
if (is_const) { continue;
}
```
--
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]
