github-actions[bot] commented on code in PR #17594:
URL: https://github.com/apache/doris/pull/17594#discussion_r1135854172
##########
be/src/vec/exec/format/parquet/fix_length_dict_decoder.hpp:
##########
@@ -31,7 +31,7 @@ class FixLengthDictDecoder final : public BaseDictDecoder {
~FixLengthDictDecoder() override = default;
Review Comment:
warning: only virtual member functions can be marked 'override'
[clang-diagnostic-error]
```suggestion
~FixLengthDictDecoder() = default;
```
##########
be/src/vec/exec/format/parquet/vparquet_group_reader.cpp:
##########
@@ -608,4 +940,117 @@ ParquetColumnReader::Statistics
RowGroupReader::statistics() {
return st;
}
+// TODO Performance Optimization
+Status RowGroupReader::_execute_conjuncts(const std::vector<VExprContext*>&
ctxs,
+ const std::vector<IColumn::Filter*>&
filters,
+ Block* block, IColumn::Filter*
result_filter,
+ bool* can_filter_all) {
+ *can_filter_all = false;
+ auto* __restrict result_filter_data = result_filter->data();
+ for (auto* ctx : ctxs) {
+ int result_column_id = -1;
+ RETURN_IF_ERROR(ctx->execute(block, &result_column_id));
+ ColumnPtr& filter_column =
block->get_by_position(result_column_id).column;
+ if (auto* nullable_column =
check_and_get_column<ColumnNullable>(*filter_column)) {
+ size_t column_size = nullable_column->size();
+ if (column_size == 0) {
+ *can_filter_all = true;
+ return Status::OK();
+ } else {
+ const ColumnPtr& nested_column =
nullable_column->get_nested_column_ptr();
+ const IColumn::Filter& filter =
+ assert_cast<const
ColumnUInt8&>(*nested_column).get_data();
+ auto* __restrict filter_data = filter.data();
+ const size_t size = filter.size();
+ auto* __restrict null_map_data =
nullable_column->get_null_map_data().data();
+
+ for (size_t i = 0; i < size; ++i) {
+ result_filter_data[i] &= (!null_map_data[i]) &
filter_data[i];
+ }
+// size_t count = size -
simd::count_zero_num((int8_t*)filter_data, size);
+// if (count == 0) {
+// *can_filter_all = true;
+// return Status::OK();
+// }
+ if (memchr(filter_data, 0x1, size) == nullptr) {
+ *can_filter_all = true;
+ return Status::OK();
+ }
+ }
+ } else if (auto* const_column =
check_and_get_column<ColumnConst>(*filter_column)) {
+ // filter all
+ if (!const_column->get_bool(0)) {
+ *can_filter_all = true;
+ return Status::OK();
+ }
+ } else {
+ const IColumn::Filter& filter =
+ assert_cast<const ColumnUInt8&>(*filter_column).get_data();
+ auto* __restrict filter_data = filter.data();
+
+ const size_t size = filter.size();
+ for (size_t i = 0; i < size; ++i) {
+ result_filter_data[i] &= filter_data[i];
+ }
+
+ if (memchr(filter_data, 0x1, size) == nullptr) {
+ *can_filter_all = true;
+ return Status::OK();
+ }
+ }
+ }
+ for (auto* filter : filters) {
+ auto* __restrict filter_data = filter->data();
+ const size_t size = filter->size();
+ for (size_t i = 0; i < size; ++i) {
+ result_filter_data[i] &= filter_data[i];
+ }
+ }
+ return Status::OK();
+}
+
+// TODO Performance Optimization
+Status RowGroupReader::_execute_conjuncts_and_filter_block(
+ const std::vector<VExprContext*>& ctxs, const
std::vector<IColumn::Filter*>& filters,
+ Block* block, std::vector<uint32_t>& columns_to_filter, int
column_to_keep) {
+ IColumn::Filter result_filter(block->rows(), 1);
+ bool can_filter_all;
+ RETURN_IF_ERROR(_execute_conjuncts(ctxs, filters, block, &result_filter,
&can_filter_all));
+ if (can_filter_all) {
+ for (auto& col : columns_to_filter) {
+
std::move(*block->get_by_position(col).column).assume_mutable()->clear();
+ }
+ } else {
+ RETURN_IF_ERROR(_filter_block_internal(block, columns_to_filter,
result_filter));
+ }
+ Block::erase_useless_column(block, column_to_keep);
+// clock_gettime(CLOCK_MONOTONIC, &endT);
+// fprintf(stderr, "==> filter_block %lu ns\n", (endT.tv_sec -
startT.tv_sec) * 1000000000 + (endT.tv_nsec - startT.tv_nsec));
+ return Status::OK();
+}
+
+// TODO Performance Optimization
+Status RowGroupReader::_execute_conjuncts_and_filter_block2(
+ const std::vector<VExprContext*>& ctxs, const
std::vector<IColumn::Filter*>& filters,
+ Block* block, std::vector<uint32_t>& columns_to_filter, int
column_to_keep) {
+ struct timespec startT, endT;
+ clock_gettime(CLOCK_MONOTONIC, &startT);
+ IColumn::Filter result_filter(block->rows(), 1);
+ bool can_filter_all;
+ RETURN_IF_ERROR(_execute_conjuncts(ctxs, filters, block, &result_filter,
&can_filter_all));
+ clock_gettime(CLOCK_MONOTONIC, &endT);
+ fprintf(stderr, "==> dict_filter_block %lu ns\n", (endT.tv_sec -
startT.tv_sec) * 1000000000 + (endT.tv_nsec - startT.tv_nsec));
+ if (can_filter_all) {
+ for (auto& col : columns_to_filter) {
+
std::move(*block->get_by_position(col).column).assume_mutable()->clear();
Review Comment:
warning: std::move of the const expression has no effect; remove std::move()
[performance-move-const-arg]
```suggestion
*block->get_by_position(col).column.assume_mutable()->clear();
```
##########
be/src/vec/functions/function.cpp:
##########
@@ -37,63 +37,52 @@
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 (auto* nullable = check_and_get_column<ColumnNullable>(*src)) {
+ if (src->only_null())
+ return src;
+ else 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.type->is_nullable()) continue;
- bool is_const = is_column_const(*elem.column);
/// Const Nullable that are NULL.
- if (is_const && assert_cast<const
ColumnConst*>(elem.column.get())->only_null()) {
+ if (elem.column->only_null())
Review Comment:
warning: statement should be inside braces
[readability-braces-around-statements]
```suggestion
if (elem.column->only_null()) {
```
be/src/vec/functions/function.cpp:57:
```diff
-
Null());
+
Null());
+ }
```
##########
be/src/vec/functions/function.cpp:
##########
@@ -37,63 +37,52 @@ 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 (auto* nullable = check_and_get_column<ColumnNullable>(*src)) {
+ if (src->only_null())
Review Comment:
warning: statement should be inside braces
[readability-braces-around-statements]
```suggestion
if (src->only_null()) {
```
be/src/vec/functions/function.cpp:45:
```diff
- else if (auto* nullable = check_and_get_column<ColumnNullable>(*src)) {
+ } else if (auto* nullable =
check_and_get_column<ColumnNullable>(*src)) {
```
##########
be/src/vec/functions/function.cpp:
##########
@@ -264,6 +258,16 @@
return execute_impl(context, block, args, result, input_rows_count);
}
+Status PreparedFunctionImpl::execute_without_low_cardinality_columns777(
+ FunctionContext* context, Block& block,
+ const ColumnNumbers& args, size_t result, size_t input_rows_count,
bool dry_run) {
+ if (dry_run)
+ return execute_impl_dry_run(context, block, args, result,
+ input_rows_count);
+ else
+ return execute_impl(context, block, args, result, input_rows_count);
Review Comment:
warning: statement should be inside braces
[readability-braces-around-statements]
```suggestion
else {
return execute_impl(context, block, args, result, input_rows_count);
}
```
##########
be/src/vec/functions/function.cpp:
##########
@@ -264,6 +258,16 @@
return execute_impl(context, block, args, result, input_rows_count);
}
+Status PreparedFunctionImpl::execute_without_low_cardinality_columns777(
+ FunctionContext* context, Block& block,
+ const ColumnNumbers& args, size_t result, size_t input_rows_count,
bool dry_run) {
+ if (dry_run)
Review Comment:
warning: statement should be inside braces
[readability-braces-around-statements]
```suggestion
if (dry_run) {
```
be/src/vec/functions/function.cpp:266:
```diff
- else
+ } else
```
##########
be/src/vec/functions/function.cpp:
##########
@@ -37,63 +37,52 @@
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 (auto* nullable = check_and_get_column<ColumnNullable>(*src)) {
+ if (src->only_null())
+ return src;
+ else 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.type->is_nullable()) continue;
- bool is_const = is_column_const(*elem.column);
/// Const Nullable that are NULL.
- if (is_const && assert_cast<const
ColumnConst*>(elem.column.get())->only_null()) {
+ if (elem.column->only_null())
return
block.get_by_position(result).type->create_column_const(input_rows_count,
Null());
- }
- if (is_const) {
- continue;
- }
- if (auto* nullable = assert_cast<const
ColumnNullable*>(elem.column.get())) {
+ if (is_column_const(*elem.column)) continue;
+
+ if (auto* nullable =
check_and_get_column<ColumnNullable>(*elem.column)) {
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(input_rows_count);
+ result_null_map_column =
null_map_column->clone_resized(null_map_column->size());
} else {
- if (!mutable_result_null_map_column) {
- mutable_result_null_map_column =
-
(*std::move(result_null_map_column)).assume_mutable();
- }
+ MutableColumnPtr 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) {
- if (is_column_const(*src)) {
- return ColumnConst::create(
- make_nullable(assert_cast<const
ColumnConst&>(*src).get_data_column_ptr(),
- false),
- input_rows_count);
- }
- return ColumnNullable::create(src,
ColumnUInt8::create(input_rows_count, 0));
- }
+ if (!result_null_map_column) return make_nullable(src);
Review Comment:
warning: statement should be inside braces
[readability-braces-around-statements]
```suggestion
if (!result_null_map_column) { return make_nullable(src);
}
```
##########
be/src/vec/exprs/vdirect_in_predicate.h:
##########
@@ -87,8 +106,14 @@ class VDirectInPredicate final : public VExpr {
std::shared_ptr<HybridSetBase> get_set_func() const override { return
_filter; }
+ void set_test_filter(std::unique_ptr<ArraySet<Int32, 2>>&& filter) {
_test_filter = std::move(filter); }
+
+ void set_test_filter2(ArraySet<Int32, 2>& filter) { _test_filter2 =
std::move(filter); }
Review Comment:
warning: std::move of the variable 'filter' of the trivially-copyable type
'ArraySet<doris::vectorized::Int32, 2>' (aka 'ArraySet<int, 2>') has no effect;
remove std::move() [performance-move-const-arg]
```suggestion
void set_test_filter2(ArraySet<Int32, 2>& filter) { _test_filter2 =
filter; }
```
##########
be/src/vec/exec/format/parquet/fix_length_dict_decoder.hpp:
##########
@@ -31,7 +31,7 @@
~FixLengthDictDecoder() override = default;
Status decode_values(MutableColumnPtr& doris_column, DataTypePtr&
data_type,
- ColumnSelectVector& select_vector) override {
+ ColumnSelectVector& select_vector, bool
is_dict_filter) override {
Review Comment:
warning: unknown type name 'ColumnSelectVector' [clang-diagnostic-error]
```cpp
ColumnSelectVector& select_vector, bool
is_dict_filter) override {
^
```
##########
be/src/vec/functions/function.cpp:
##########
@@ -37,63 +37,52 @@
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 (auto* nullable = check_and_get_column<ColumnNullable>(*src)) {
+ if (src->only_null())
+ return src;
+ else 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.type->is_nullable()) continue;
- bool is_const = is_column_const(*elem.column);
/// Const Nullable that are NULL.
- if (is_const && assert_cast<const
ColumnConst*>(elem.column.get())->only_null()) {
+ if (elem.column->only_null())
return
block.get_by_position(result).type->create_column_const(input_rows_count,
Null());
- }
- if (is_const) {
- continue;
- }
- if (auto* nullable = assert_cast<const
ColumnNullable*>(elem.column.get())) {
+ if (is_column_const(*elem.column)) continue;
Review Comment:
warning: statement should be inside braces
[readability-braces-around-statements]
```suggestion
if (is_column_const(*elem.column)) { continue;
}
```
##########
be/src/vec/functions/function.cpp:
##########
@@ -37,63 +37,52 @@
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 (auto* nullable = check_and_get_column<ColumnNullable>(*src)) {
+ if (src->only_null())
+ return src;
+ else 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.type->is_nullable()) continue;
Review Comment:
warning: statement should be inside braces
[readability-braces-around-statements]
```suggestion
if (!elem.type->is_nullable()) { 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]
