github-actions[bot] commented on code in PR #15605:
URL: https://github.com/apache/doris/pull/15605#discussion_r1061246820


##########
be/src/vec/exec/join/vjoin_node_base.cpp:
##########
@@ -107,46 +107,51 @@ void VJoinNodeBase::_construct_mutable_join_block() {
 
 Status VJoinNodeBase::_build_output_block(Block* origin_block, Block* 
output_block) {
     auto is_mem_reuse = output_block->mem_reuse();
-    MutableBlock mutable_block =
-            is_mem_reuse
-                    ? MutableBlock(output_block)
-                    : 
MutableBlock(VectorizedUtils::create_empty_columnswithtypename(row_desc()));
     auto rows = origin_block->rows();
     // TODO: After FE plan support same nullable of output expr and origin 
block and mutable column
     // we should replace `insert_column_datas` by `insert_range_from`
-
-    auto insert_column_datas = [](auto& to, const auto& from, size_t rows) {
-        if (to->is_nullable() && !from.is_nullable()) {
-            auto& null_column = reinterpret_cast<ColumnNullable&>(*to);
-            null_column.get_nested_column().insert_range_from(from, 0, rows);
-            null_column.get_null_map_column().get_data().resize_fill(rows, 0);
-        } else {
-            to->insert_range_from(from, 0, rows);
-        }
-    };
+    // Currently, this is only a test feture, because the nullable is not the 
same, so that BE may core.
     if (rows != 0) {
-        auto& mutable_columns = mutable_block.mutable_columns();
+        SCOPED_TIMER(_projection_timer);
+        MutableColumns mutable_columns;
+        size_t output_size = row_desc().num_materialized_slots();
         if (_output_expr_ctxs.empty()) {
-            DCHECK(mutable_columns.size() == 
row_desc().num_materialized_slots());
-            for (int i = 0; i < mutable_columns.size(); ++i) {
-                insert_column_datas(mutable_columns[i], 
*origin_block->get_by_position(i).column,
-                                    rows);
+            for (int i = 0; i < output_size; ++i) {
+                mutable_columns.emplace_back(
+                        
std::move(*origin_block->get_by_position(i).column).mutate());

Review Comment:
   warning: std::move of the const expression has no effect; remove std::move() 
[performance-move-const-arg]
   
   ```suggestion
                           *origin_block->get_by_position(i).column.mutate());
   ```
   



##########
be/src/vec/exec/join/vjoin_node_base.cpp:
##########
@@ -107,46 +107,51 @@
 
 Status VJoinNodeBase::_build_output_block(Block* origin_block, Block* 
output_block) {
     auto is_mem_reuse = output_block->mem_reuse();
-    MutableBlock mutable_block =
-            is_mem_reuse
-                    ? MutableBlock(output_block)
-                    : 
MutableBlock(VectorizedUtils::create_empty_columnswithtypename(row_desc()));
     auto rows = origin_block->rows();
     // TODO: After FE plan support same nullable of output expr and origin 
block and mutable column
     // we should replace `insert_column_datas` by `insert_range_from`
-
-    auto insert_column_datas = [](auto& to, const auto& from, size_t rows) {
-        if (to->is_nullable() && !from.is_nullable()) {
-            auto& null_column = reinterpret_cast<ColumnNullable&>(*to);
-            null_column.get_nested_column().insert_range_from(from, 0, rows);
-            null_column.get_null_map_column().get_data().resize_fill(rows, 0);
-        } else {
-            to->insert_range_from(from, 0, rows);
-        }
-    };
+    // Currently, this is only a test feture, because the nullable is not the 
same, so that BE may core.
     if (rows != 0) {
-        auto& mutable_columns = mutable_block.mutable_columns();
+        SCOPED_TIMER(_projection_timer);
+        MutableColumns mutable_columns;
+        size_t output_size = row_desc().num_materialized_slots();
         if (_output_expr_ctxs.empty()) {
-            DCHECK(mutable_columns.size() == 
row_desc().num_materialized_slots());
-            for (int i = 0; i < mutable_columns.size(); ++i) {
-                insert_column_datas(mutable_columns[i], 
*origin_block->get_by_position(i).column,
-                                    rows);
+            for (int i = 0; i < output_size; ++i) {
+                mutable_columns.emplace_back(
+                        
std::move(*origin_block->get_by_position(i).column).mutate());
             }
         } else {
-            DCHECK(mutable_columns.size() == 
row_desc().num_materialized_slots());
-            SCOPED_TIMER(_projection_timer);
-            for (int i = 0; i < mutable_columns.size(); ++i) {
-                auto result_column_id = -1;
+            for (int i = 0; i < output_size; ++i) {
+                int result_column_id = -1;
                 RETURN_IF_ERROR(_output_expr_ctxs[i]->execute(origin_block, 
&result_column_id));
                 auto column_ptr = 
origin_block->get_by_position(result_column_id)
                                           
.column->convert_to_full_column_if_const();
-                insert_column_datas(mutable_columns[i], *column_ptr, rows);
+                mutable_columns.emplace_back(std::move(*column_ptr).mutate());

Review Comment:
   warning: std::move of the const expression has no effect; remove std::move() 
[performance-move-const-arg]
   
   ```suggestion
                   mutable_columns.emplace_back(*column_ptr.mutate());
   ```
   



-- 
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: commits-unsubscr...@doris.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org

Reply via email to