From b5d8ffa295f34cc86856dc4d662234566000ade8 Mon Sep 17 00:00:00 2001 From: HappenLee Date: Sun, 27 Oct 2024 04:32:28 +0800 Subject: [PATCH] [Refactor](exec) Refactor some unreasonable code in exec --- be/src/pipeline/exec/hashjoin_build_sink.h | 3 ++- be/src/pipeline/exec/hashjoin_probe_operator.cpp | 2 +- be/src/vec/core/block.cpp | 4 ++-- be/src/vec/core/block.h | 2 +- be/src/vec/sink/vrow_distribution.cpp | 4 ++-- be/src/vec/sink/vrow_distribution.h | 2 +- .../rules/expression/rules/NullSafeEqualToEqualTest.java | 2 +- 7 files changed, 10 insertions(+), 9 deletions(-) diff --git a/be/src/pipeline/exec/hashjoin_build_sink.h b/be/src/pipeline/exec/hashjoin_build_sink.h index 69aa6843b84ecb..f69d14a1081efe 100644 --- a/be/src/pipeline/exec/hashjoin_build_sink.h +++ b/be/src/pipeline/exec/hashjoin_build_sink.h @@ -193,6 +193,7 @@ struct ProcessHashTableBuild { bool* has_null_key) { if (short_circuit_for_null || ignore_null) { // first row is mocked and is null + // TODO: Need to test the for loop. break may better for (uint32_t i = 1; i < _rows; i++) { if ((*null_map)[i]) { *has_null_key = true; @@ -231,4 +232,4 @@ struct ProcessHashTableBuild { }; } // namespace doris::pipeline -#include "common/compile_check_end.h" \ No newline at end of file +#include "common/compile_check_end.h" diff --git a/be/src/pipeline/exec/hashjoin_probe_operator.cpp b/be/src/pipeline/exec/hashjoin_probe_operator.cpp index 8ee041f57592ca..904c95282e21fc 100644 --- a/be/src/pipeline/exec/hashjoin_probe_operator.cpp +++ b/be/src/pipeline/exec/hashjoin_probe_operator.cpp @@ -154,7 +154,7 @@ bool HashJoinProbeLocalState::_need_probe_null_map(vectorized::Block& block, const std::vector& res_col_ids) { for (size_t i = 0; i < _probe_expr_ctxs.size(); ++i) { if (!_shared_state->is_null_safe_eq_join[i]) { - auto column = block.get_by_position(res_col_ids[i]).column.get(); + const auto* column = block.get_by_position(res_col_ids[i]).column.get(); if (check_and_get_column(*column)) { return true; } diff --git a/be/src/vec/core/block.cpp b/be/src/vec/core/block.cpp index 2eb06e3c6a553e..11075335fb17af 100644 --- a/be/src/vec/core/block.cpp +++ b/be/src/vec/core/block.cpp @@ -1083,7 +1083,7 @@ Status MutableBlock::add_rows(const Block* block, size_t row_begin, size_t lengt return Status::OK(); } -Status MutableBlock::add_rows(const Block* block, std::vector rows) { +Status MutableBlock::add_rows(const Block* block, const std::vector& rows) { RETURN_IF_CATCH_EXCEPTION({ DCHECK_LE(columns(), block->columns()); const auto& block_data = block->get_columns_with_type_and_name(); @@ -1093,7 +1093,7 @@ Status MutableBlock::add_rows(const Block* block, std::vector rows) { auto& dst = _columns[i]; const auto& src = *block_data[i].column.get(); dst->reserve(dst->size() + length); - for (size_t row : rows) { + for (auto row : rows) { // we can introduce a new function like `insert_assume_reserved` for IColumn. dst->insert_from(src, row); } diff --git a/be/src/vec/core/block.h b/be/src/vec/core/block.h index bbcdd9472ae178..d1af45e1297d4f 100644 --- a/be/src/vec/core/block.h +++ b/be/src/vec/core/block.h @@ -624,7 +624,7 @@ class MutableBlock { Status add_rows(const Block* block, const uint32_t* row_begin, const uint32_t* row_end, const std::vector* column_offset = nullptr); Status add_rows(const Block* block, size_t row_begin, size_t length); - Status add_rows(const Block* block, std::vector rows); + Status add_rows(const Block* block, const std::vector& rows); /// remove the column with the specified name void erase(const String& name); diff --git a/be/src/vec/sink/vrow_distribution.cpp b/be/src/vec/sink/vrow_distribution.cpp index 3a4c7e911f4c14..3d8ea8b7594c28 100644 --- a/be/src/vec/sink/vrow_distribution.cpp +++ b/be/src/vec/sink/vrow_distribution.cpp @@ -50,7 +50,7 @@ VRowDistribution::_get_partition_function() { Status VRowDistribution::_save_missing_values( std::vector>& col_strs, // non-const ref for move - int col_size, Block* block, std::vector filter, + int col_size, Block* block, const std::vector& filter, const std::vector& col_null_maps) { // de-duplication for new partitions but save all rows. RETURN_IF_ERROR(_batching_block->add_rows(block, filter)); @@ -311,7 +311,7 @@ Status VRowDistribution::_generate_rows_distribution_for_auto_partition( auto num_rows = block->rows(); std::vector partition_keys = _vpartition->get_partition_keys(); - auto partition_col = block->get_by_position(partition_keys[0]); + auto& partition_col = block->get_by_position(partition_keys[0]); _missing_map.clear(); _missing_map.reserve(partition_col.column->size()); bool stop_processing = false; diff --git a/be/src/vec/sink/vrow_distribution.h b/be/src/vec/sink/vrow_distribution.h index fffe0e3f7f1887..75cf4937312379 100644 --- a/be/src/vec/sink/vrow_distribution.h +++ b/be/src/vec/sink/vrow_distribution.h @@ -143,7 +143,7 @@ class VRowDistribution { std::pair _get_partition_function(); Status _save_missing_values(std::vector>& col_strs, int col_size, - Block* block, std::vector filter, + Block* block, const std::vector& filter, const std::vector& col_null_maps); void _get_tablet_ids(vectorized::Block* block, int32_t index_idx, diff --git a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/expression/rules/NullSafeEqualToEqualTest.java b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/expression/rules/NullSafeEqualToEqualTest.java index 8da25e92e7eec7..d4adc821880b60 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/expression/rules/NullSafeEqualToEqualTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/expression/rules/NullSafeEqualToEqualTest.java @@ -54,7 +54,7 @@ void testNullSafeEqualToFalse() { assertRewrite(new NullSafeEqual(new IntegerLiteral(0), NullLiteral.INSTANCE), BooleanLiteral.FALSE); } - // "NULL <=> Null" to false + // "NULL <=> Null" to true @Test void testNullSafeEqualToTrue() { executor = new ExpressionRuleExecutor(ImmutableList.of(