github-actions[bot] commented on code in PR #17192:
URL: https://github.com/apache/doris/pull/17192#discussion_r1118449426
##########
be/src/vec/core/block.cpp:
##########
@@ -177,15 +177,26 @@ void Block::erase(const std::set<size_t>& positions) {
}
}
-void Block::erase(size_t position) {
- if (data.empty()) {
- LOG(FATAL) << "Block is empty";
+void Block::erase_tail(size_t start) {
+ DCHECK(start < data.size()) << fmt::format(
+ "Position out of bound in Block::erase(), max position = {}",
data.size() - 1);
+ data.erase(data.begin() + start, data.end());
+ for (auto it = index_by_name.begin(); it != index_by_name.end();) {
+ if (it->second >= start)
Review Comment:
warning: statement should be inside braces
[readability-braces-around-statements]
```suggestion
if (it->second >= start) {
```
be/src/vec/core/block.cpp:186:
```diff
- else {
+ } else {
```
##########
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())
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;
- 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/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]
