From 4924ac1ff2d630b3b4e637a71fb231fc40058b45 Mon Sep 17 00:00:00 2001 From: Mryange <2319153948@qq.com> Date: Mon, 25 Mar 2024 23:15:27 +0800 Subject: [PATCH 1/2] fix --- be/src/exec/exec_node.h | 6 +++--- be/src/pipeline/exec/join_probe_operator.cpp | 4 +--- be/src/pipeline/pipeline_x/operator.cpp | 2 +- be/src/pipeline/pipeline_x/operator.h | 4 ++-- be/src/vec/exec/join/vjoin_node_base.cpp | 3 +-- 5 files changed, 8 insertions(+), 11 deletions(-) diff --git a/be/src/exec/exec_node.h b/be/src/exec/exec_node.h index 5d7b3a91651757..f2303068437b2f 100644 --- a/be/src/exec/exec_node.h +++ b/be/src/exec/exec_node.h @@ -127,7 +127,7 @@ class ExecNode { bool has_output_row_descriptor() const { return _output_row_descriptor != nullptr; } // If use projection, we should clear `_origin_block`. void clear_origin_block() { - _origin_block.clear_column_data(_row_descriptor.num_materialized_slots()); + _origin_block.clear_column_data(intermediate_row_desc().num_materialized_slots()); } // Emit data, both need impl with method: sink @@ -326,8 +326,8 @@ class ExecNode { std::shared_ptr _query_statistics = nullptr; //_keep_origin is used to avoid copying during projection, - // currently set to true only in the nestloop join. - bool _keep_origin = false; + // currently set to false only in the nestloop join. + bool _keep_origin = true; private: static Status create_tree_helper(RuntimeState* state, ObjectPool* pool, diff --git a/be/src/pipeline/exec/join_probe_operator.cpp b/be/src/pipeline/exec/join_probe_operator.cpp index 5c89075e8b9a98..c78e5423709cf5 100644 --- a/be/src/pipeline/exec/join_probe_operator.cpp +++ b/be/src/pipeline/exec/join_probe_operator.cpp @@ -87,9 +87,7 @@ Status JoinProbeLocalState::_build_output_block( // and you could see a 'todo' in the Thrift definition. // Here, we have refactored it, but considering upgrade compatibility, we still need to retain the old code. if (!output_block->mem_reuse()) { - vectorized::MutableBlock tmp( - vectorized::VectorizedUtils::create_columns_with_type_and_name(p.row_desc())); - output_block->swap(tmp.to_block()); + output_block->swap(origin_block->clone_empty()); } output_block->swap(*origin_block); return Status::OK(); diff --git a/be/src/pipeline/pipeline_x/operator.cpp b/be/src/pipeline/pipeline_x/operator.cpp index 6ee9ccb13c4d89..b19a6e48298168 100644 --- a/be/src/pipeline/pipeline_x/operator.cpp +++ b/be/src/pipeline/pipeline_x/operator.cpp @@ -167,7 +167,7 @@ Status OperatorXBase::close(RuntimeState* state) { } void PipelineXLocalStateBase::clear_origin_block() { - _origin_block.clear_column_data(_parent->_row_descriptor.num_materialized_slots()); + _origin_block.clear_column_data(_parent->intermediate_row_desc().num_materialized_slots()); } Status OperatorXBase::do_projections(RuntimeState* state, vectorized::Block* origin_block, diff --git a/be/src/pipeline/pipeline_x/operator.h b/be/src/pipeline/pipeline_x/operator.h index 56991d43105fd5..c375efb924dcbc 100644 --- a/be/src/pipeline/pipeline_x/operator.h +++ b/be/src/pipeline/pipeline_x/operator.h @@ -328,8 +328,8 @@ class OperatorXBase : public OperatorBase { int _parallel_tasks = 0; //_keep_origin is used to avoid copying during projection, - // currently set to true only in the nestloop join. - bool _keep_origin = false; + // currently set to false only in the nestloop join. + bool _keep_origin = true; }; template diff --git a/be/src/vec/exec/join/vjoin_node_base.cpp b/be/src/vec/exec/join/vjoin_node_base.cpp index 9b954811ee9a02..a9e25e7626bbd8 100644 --- a/be/src/vec/exec/join/vjoin_node_base.cpp +++ b/be/src/vec/exec/join/vjoin_node_base.cpp @@ -185,8 +185,7 @@ Status VJoinNodeBase::_build_output_block(Block* origin_block, Block* output_blo // and you could see a 'todo' in the Thrift definition. // Here, we have refactored it, but considering upgrade compatibility, we still need to retain the old code. if (!output_block->mem_reuse()) { - MutableBlock tmp(VectorizedUtils::create_columns_with_type_and_name(row_desc())); - output_block->swap(tmp.to_block()); + output_block->swap(origin_block->clone_empty()); } output_block->swap(*origin_block); return Status::OK(); From ee26304ef83b2a66471f01d77f871dbb2c10264f Mon Sep 17 00:00:00 2001 From: Mryange <2319153948@qq.com> Date: Tue, 26 Mar 2024 15:09:44 +0800 Subject: [PATCH 2/2] add case --- .../data/correctness_p0/test_probe_clean.out | 10 ++ .../correctness_p0/test_probe_clean.groovy | 95 +++++++++++++++++++ 2 files changed, 105 insertions(+) create mode 100644 regression-test/data/correctness_p0/test_probe_clean.out create mode 100644 regression-test/suites/correctness_p0/test_probe_clean.groovy diff --git a/regression-test/data/correctness_p0/test_probe_clean.out b/regression-test/data/correctness_p0/test_probe_clean.out new file mode 100644 index 00000000000000..78ab5a2b890be1 --- /dev/null +++ b/regression-test/data/correctness_p0/test_probe_clean.out @@ -0,0 +1,10 @@ +-- This file is automatically generated. You should know what you did if you want to edit this +-- !select_pipelineX -- +2020 -5.2 + +-- !select_pipeline -- +2020 -5.2 + +-- !select_non_pipeline -- +2020 -5.2 + diff --git a/regression-test/suites/correctness_p0/test_probe_clean.groovy b/regression-test/suites/correctness_p0/test_probe_clean.groovy new file mode 100644 index 00000000000000..febc05f66fbe02 --- /dev/null +++ b/regression-test/suites/correctness_p0/test_probe_clean.groovy @@ -0,0 +1,95 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +// The cases is copied from https://github.com/trinodb/trino/tree/master +// /testing/trino-product-tests/src/main/resources/sql-tests/testcases/aggregate +// and modified by Doris. + +suite("test_probe_clean") { + +sql """ drop table IF EXISTS clearblocktable1; """ +sql """ + CREATE TABLE IF NOT EXISTS clearblocktable1 ( + `col_int_undef_signed` INT NULL COMMENT "", + `col_int_undef_signed_not_null` INT NOT NULL COMMENT "", + `col_date_undef_signed_not_null` date(11) NOT NULL COMMENT "", + + ) ENGINE=OLAP + DUPLICATE KEY(`col_int_undef_signed`) + DISTRIBUTED BY HASH(`col_int_undef_signed`) BUCKETS 1 + PROPERTIES ( + 'replication_num' = '1' +); +""" + + +sql """ +insert into clearblocktable1 values(1,1,'2020-01-01'); +""" +sql """ +drop table IF EXISTS clearblocktable2; +""" +sql """ +CREATE TABLE IF NOT EXISTS clearblocktable2 ( + `col_int_undef_signed` INT NULL COMMENT "", + `col_int_undef_signed_not_null` INT NOT NULL COMMENT "", + `col_date_undef_signed_not_null` date(11) NOT NULL COMMENT "", + + ) ENGINE=OLAP + DUPLICATE KEY(`col_int_undef_signed`) + DISTRIBUTED BY HASH(`col_int_undef_signed`) BUCKETS 1 + PROPERTIES ( + 'replication_num' = '1' +); +""" + +sql """ +insert into clearblocktable2 values(1,1,'2020-01-01'); +""" + +sql """ +set enable_pipeline_x_engine=true, enable_pipeline_engine=true; +""" +qt_select_pipelineX """ + +SELECT YEAR(ifnull(clearblocktable1.`col_date_undef_signed_not_null`, clearblocktable1.`col_date_undef_signed_not_null`)) AS field1 , +CASE WHEN clearblocktable1.`col_int_undef_signed` != clearblocktable1.`col_int_undef_signed` * (8 + 1) THEN -5.2 ELSE clearblocktable1.`col_int_undef_signed` END AS field2 +FROM clearblocktable1 INNER JOIN clearblocktable2 ON clearblocktable2.`col_int_undef_signed` = clearblocktable1.`col_int_undef_signed` WHERE clearblocktable1.`col_int_undef_signed_not_null` <> 7; + +""" + +sql """ +set enable_pipeline_x_engine=false,enable_pipeline_engine=true; +""" +qt_select_pipeline """ + +SELECT YEAR(ifnull(clearblocktable1.`col_date_undef_signed_not_null`, clearblocktable1.`col_date_undef_signed_not_null`)) AS field1 , +CASE WHEN clearblocktable1.`col_int_undef_signed` != clearblocktable1.`col_int_undef_signed` * (8 + 1) THEN -5.2 ELSE clearblocktable1.`col_int_undef_signed` END AS field2 +FROM clearblocktable1 INNER JOIN clearblocktable2 ON clearblocktable2.`col_int_undef_signed` = clearblocktable1.`col_int_undef_signed` WHERE clearblocktable1.`col_int_undef_signed_not_null` <> 7; + +""" + +sql """ +set enable_pipeline_x_engine=false, enable_pipeline_engine=false; +""" +qt_select_non_pipeline """ + +SELECT YEAR(ifnull(clearblocktable1.`col_date_undef_signed_not_null`, clearblocktable1.`col_date_undef_signed_not_null`)) AS field1 , +CASE WHEN clearblocktable1.`col_int_undef_signed` != clearblocktable1.`col_int_undef_signed` * (8 + 1) THEN -5.2 ELSE clearblocktable1.`col_int_undef_signed` END AS field2 +FROM clearblocktable1 INNER JOIN clearblocktable2 ON clearblocktable2.`col_int_undef_signed` = clearblocktable1.`col_int_undef_signed` WHERE clearblocktable1.`col_int_undef_signed_not_null` <> 7; +""" +}