From d7d0d7796337b4412ab7476bcdcd671ff67acded Mon Sep 17 00:00:00 2001 From: starocean999 <12095047@qq.com> Date: Tue, 18 Oct 2022 15:42:31 +0800 Subject: [PATCH 1/2] [fix](join)the build and probe expr should be calculated before converting input block to nullable --- be/src/vec/columns/column_const.h | 2 +- be/src/vec/exec/join/vhash_join_node.cpp | 13 ++- .../test_outer_join_with_subquery.out | 4 + .../test_outer_join_with_subquery.groovy | 88 +++++++++++++++++++ 4 files changed, 99 insertions(+), 8 deletions(-) create mode 100644 regression-test/data/correctness_p0/test_outer_join_with_subquery.out create mode 100644 regression-test/suites/correctness_p0/test_outer_join_with_subquery.groovy diff --git a/be/src/vec/columns/column_const.h b/be/src/vec/columns/column_const.h index 9637a0943f0799..5a440a6695d260 100644 --- a/be/src/vec/columns/column_const.h +++ b/be/src/vec/columns/column_const.h @@ -185,7 +185,7 @@ class ColumnConst final : public COWHelper { return false; } - // bool is_nullable() const override { return is_column_nullable(*data); } + bool is_nullable() const override { return data->is_nullable(); } bool only_null() const override { return data->is_null_at(0); } bool is_numeric() const override { return data->is_numeric(); } bool is_fixed_and_contiguous() const override { return data->is_fixed_and_contiguous(); } diff --git a/be/src/vec/exec/join/vhash_join_node.cpp b/be/src/vec/exec/join/vhash_join_node.cpp index fad0eb68cfb786..74a34311626ba5 100644 --- a/be/src/vec/exec/join/vhash_join_node.cpp +++ b/be/src/vec/exec/join/vhash_join_node.cpp @@ -1038,10 +1038,6 @@ Status HashJoinNode::get_next(RuntimeState* state, Block* output_block, bool* eo probe_rows = _probe_block.rows(); if (probe_rows != 0) { COUNTER_UPDATE(_probe_rows_counter, probe_rows); - if (_join_op == TJoinOp::RIGHT_OUTER_JOIN || _join_op == TJoinOp::FULL_OUTER_JOIN) { - _probe_column_convert_to_null = _convert_block_to_null(_probe_block); - } - int probe_expr_ctxs_sz = _probe_expr_ctxs.size(); _probe_columns.resize(probe_expr_ctxs_sz); @@ -1075,6 +1071,9 @@ Status HashJoinNode::get_next(RuntimeState* state, Block* output_block, bool* eo _hash_table_variants); RETURN_IF_ERROR(st); + if (_join_op == TJoinOp::RIGHT_OUTER_JOIN || _join_op == TJoinOp::FULL_OUTER_JOIN) { + _probe_column_convert_to_null = _convert_block_to_null(_probe_block); + } } } @@ -1378,9 +1377,6 @@ bool HashJoinNode::_need_null_map(Block& block, const std::vector& res_col_ Status HashJoinNode::_process_build_block(RuntimeState* state, Block& block, uint8_t offset) { SCOPED_TIMER(_build_table_timer); - if (_join_op == TJoinOp::LEFT_OUTER_JOIN || _join_op == TJoinOp::FULL_OUTER_JOIN) { - _convert_block_to_null(block); - } size_t rows = block.rows(); if (UNLIKELY(rows == 0)) { return Status::OK(); @@ -1436,6 +1432,9 @@ Status HashJoinNode::_process_build_block(RuntimeState* state, Block& block, uin make_bool_variant(_build_unique), make_bool_variant(has_runtime_filter), make_bool_variant(_need_null_map_for_build)); + if (_join_op == TJoinOp::LEFT_OUTER_JOIN || _join_op == TJoinOp::FULL_OUTER_JOIN) { + _convert_block_to_null(block); + } return st; } diff --git a/regression-test/data/correctness_p0/test_outer_join_with_subquery.out b/regression-test/data/correctness_p0/test_outer_join_with_subquery.out new file mode 100644 index 00000000000000..72d126351a3050 --- /dev/null +++ b/regression-test/data/correctness_p0/test_outer_join_with_subquery.out @@ -0,0 +1,4 @@ +-- This file is automatically generated. You should know what you did if you want to edit this +-- !select -- +1 + diff --git a/regression-test/suites/correctness_p0/test_outer_join_with_subquery.groovy b/regression-test/suites/correctness_p0/test_outer_join_with_subquery.groovy new file mode 100644 index 00000000000000..59bb822e3b13b2 --- /dev/null +++ b/regression-test/suites/correctness_p0/test_outer_join_with_subquery.groovy @@ -0,0 +1,88 @@ +// 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. + +suite("test_outer_join_with_subquery") { + sql """ + drop table if exists test_outer_join_with_subquery_outerjoin_A; + """ + + sql """ + drop table if exists test_outer_join_with_subquery_outerjoin_B; + """ + + sql """ + create table test_outer_join_with_subquery_outerjoin_A ( a int not null ) + ENGINE=OLAP + DISTRIBUTED BY HASH(a) BUCKETS 1 + PROPERTIES ( + "replication_allocation" = "tag.location.default: 1", + "in_memory" = "false", + "storage_format" = "V2" + ); + """ + + sql """ + create table test_outer_join_with_subquery_outerjoin_B ( a int not null ) + ENGINE=OLAP + DISTRIBUTED BY HASH(a) BUCKETS 1 + PROPERTIES ( + "replication_allocation" = "tag.location.default: 1", + "in_memory" = "false", + "storage_format" = "V2" + ); + """ + + sql """ + insert into test_outer_join_with_subquery_outerjoin_A values( 1 ); + """ + + sql """ + insert into test_outer_join_with_subquery_outerjoin_B values( 1 ); + """ + + qt_select """ + select + subq_1.c1 + from + ( + select + case + when test_outer_join_with_subquery_outerjoin_A.a is NULL then test_outer_join_with_subquery_outerjoin_A.a + else test_outer_join_with_subquery_outerjoin_A.a + end as c1 + from + test_outer_join_with_subquery_outerjoin_A + ) as subq_1 + full join ( + select + case + when test_outer_join_with_subquery_outerjoin_B.a is NULL then test_outer_join_with_subquery_outerjoin_B.a + else test_outer_join_with_subquery_outerjoin_B.a + end as c2 + from + test_outer_join_with_subquery_outerjoin_B + ) as subq_2 on (subq_1.c1 = subq_2.c2); + """ + + sql """ + drop table if exists test_outer_join_with_subquery_outerjoin_A; + """ + + sql """ + drop table if exists test_outer_join_with_subquery_outerjoin_B; + """ +} From be71bdfbdea8cf307d778fc18e3591d169781f2d Mon Sep 17 00:00:00 2001 From: starocean999 <12095047@qq.com> Date: Fri, 21 Oct 2022 18:16:32 +0800 Subject: [PATCH 2/2] remove_nullable can be called on const column --- be/src/vec/columns/column_const.h | 2 +- be/src/vec/exec/join/vhash_join_node.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/be/src/vec/columns/column_const.h b/be/src/vec/columns/column_const.h index 5a440a6695d260..9637a0943f0799 100644 --- a/be/src/vec/columns/column_const.h +++ b/be/src/vec/columns/column_const.h @@ -185,7 +185,7 @@ class ColumnConst final : public COWHelper { return false; } - bool is_nullable() const override { return data->is_nullable(); } + // bool is_nullable() const override { return is_column_nullable(*data); } bool only_null() const override { return data->is_null_at(0); } bool is_numeric() const override { return data->is_numeric(); } bool is_fixed_and_contiguous() const override { return data->is_fixed_and_contiguous(); } diff --git a/be/src/vec/exec/join/vhash_join_node.cpp b/be/src/vec/exec/join/vhash_join_node.cpp index 74a34311626ba5..f94c66b9537bd2 100644 --- a/be/src/vec/exec/join/vhash_join_node.cpp +++ b/be/src/vec/exec/join/vhash_join_node.cpp @@ -1175,7 +1175,7 @@ void HashJoinNode::_prepare_probe_block() { // remove add nullmap of probe columns for (auto index : _probe_column_convert_to_null) { auto& column_type = _probe_block.safe_get_by_position(index); - DCHECK(column_type.column->is_nullable()); + DCHECK(column_type.column->is_nullable() || is_column_const(*(column_type.column.get()))); DCHECK(column_type.type->is_nullable()); column_type.column = remove_nullable(column_type.column);