Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "[Improvementation](join) empty_block shall be set true when b… #33916

Merged
merged 1 commit into from
Apr 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 8 additions & 9 deletions be/src/pipeline/exec/hashjoin_build_sink.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -155,22 +155,21 @@ bool HashJoinBuildSinkLocalState::build_unique() const {

void HashJoinBuildSinkLocalState::init_short_circuit_for_probe() {
auto& p = _parent->cast<HashJoinBuildSinkOperatorX>();
bool empty_block =
!_shared_state->build_block ||
!(_shared_state->build_block->rows() > 1); // build size always mock a row into block
_shared_state->short_circuit_for_probe =
(_shared_state->_has_null_in_build_side &&
p._join_op == TJoinOp::NULL_AWARE_LEFT_ANTI_JOIN && !p._is_mark_join) ||
(empty_block && p._join_op == TJoinOp::INNER_JOIN && !p._is_mark_join) ||
(empty_block && p._join_op == TJoinOp::LEFT_SEMI_JOIN && !p._is_mark_join) ||
(empty_block && p._join_op == TJoinOp::RIGHT_OUTER_JOIN) ||
(empty_block && p._join_op == TJoinOp::RIGHT_SEMI_JOIN) ||
(empty_block && p._join_op == TJoinOp::RIGHT_ANTI_JOIN);
(!_shared_state->build_block && p._join_op == TJoinOp::INNER_JOIN &&
!p._is_mark_join) ||
(!_shared_state->build_block && p._join_op == TJoinOp::LEFT_SEMI_JOIN &&
!p._is_mark_join) ||
(!_shared_state->build_block && p._join_op == TJoinOp::RIGHT_OUTER_JOIN) ||
(!_shared_state->build_block && p._join_op == TJoinOp::RIGHT_SEMI_JOIN) ||
(!_shared_state->build_block && p._join_op == TJoinOp::RIGHT_ANTI_JOIN);

//when build table rows is 0 and not have other_join_conjunct and not _is_mark_join and join type is one of LEFT_OUTER_JOIN/FULL_OUTER_JOIN/LEFT_ANTI_JOIN
//we could get the result is probe table + null-column(if need output)
_shared_state->empty_right_table_need_probe_dispose =
(empty_block && !p._have_other_join_conjunct && !p._is_mark_join) &&
(!_shared_state->build_block && !p._have_other_join_conjunct && !p._is_mark_join) &&
(p._join_op == TJoinOp::LEFT_OUTER_JOIN || p._join_op == TJoinOp::FULL_OUTER_JOIN ||
p._join_op == TJoinOp::LEFT_ANTI_JOIN);
}
Expand Down
17 changes: 11 additions & 6 deletions be/src/pipeline/exec/hashjoin_probe_operator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -247,9 +247,7 @@ Status HashJoinProbeOperatorX::pull(doris::RuntimeState* state, vectorized::Bloc
}

//TODO: this short circuit maybe could refactor, no need to check at here.
// only support nereids
if (local_state._shared_state->empty_right_table_need_probe_dispose &&
!Base::_projections.empty()) {
if (local_state._shared_state->empty_right_table_need_probe_dispose) {
// when build table rows is 0 and not have other_join_conjunct and join type is one of LEFT_OUTER_JOIN/FULL_OUTER_JOIN/LEFT_ANTI_JOIN
// we could get the result is probe table + null-column(if need output)
// If we use a short-circuit strategy, should return block directly by add additional null data.
Expand All @@ -259,6 +257,12 @@ Status HashJoinProbeOperatorX::pull(doris::RuntimeState* state, vectorized::Bloc
return Status::OK();
}

vectorized::Block temp_block;
//get probe side output column
for (int i = 0; i < _left_output_slot_flags.size(); ++i) {
temp_block.insert(local_state._probe_block.get_by_position(i));
}

//create build side null column, if need output
for (int i = 0;
(_join_op != TJoinOp::LEFT_ANTI_JOIN) && i < _right_output_slot_flags.size(); ++i) {
Expand All @@ -269,8 +273,8 @@ Status HashJoinProbeOperatorX::pull(doris::RuntimeState* state, vectorized::Bloc
vectorized::ColumnVector<vectorized::UInt8>::create(block_rows, 1);
auto nullable_column = vectorized::ColumnNullable::create(std::move(column),
std::move(null_map_column));
local_state._probe_block.insert({std::move(nullable_column), make_nullable(type),
_right_table_column_names[i]});
temp_block.insert({std::move(nullable_column), make_nullable(type),
_right_table_column_names[i]});
}
if (_is_outer_join) {
reinterpret_cast<vectorized::ColumnUInt8*>(
Expand All @@ -286,7 +290,8 @@ Status HashJoinProbeOperatorX::pull(doris::RuntimeState* state, vectorized::Bloc
/// No need to check the block size in `_filter_data_and_build_output` because here dose not
/// increase the output rows count(just same as `_probe_block`'s rows count).
RETURN_IF_ERROR(local_state.filter_data_and_build_output(state, output_block, eos,
&local_state._probe_block, false));
&temp_block, false));
temp_block.clear();
local_state._probe_block.clear_column_data(_child_x->row_desc().num_materialized_slots());
return Status::OK();
}
Expand Down
12 changes: 5 additions & 7 deletions be/src/vec/core/column_with_type_and_name.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -62,17 +62,15 @@ void ColumnWithTypeAndName::dump_structure(std::ostream& out) const {
out << name;
}

if (type) {
if (type)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: statement should be inside braces [readability-braces-around-statements]

Suggested change
if (type)
if (type) {

be/src/vec/core/column_with_type_and_name.cpp:66:

-     else
+     } else

out << " " << type->get_name();
} else {
else
out << " nullptr";
Comment on lines +67 to 68
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: statement should be inside braces [readability-braces-around-statements]

Suggested change
else
out << " nullptr";
else {
out << " nullptr";
}

}

if (column) {
out << ' ' << column->dump_structure() << "(use_count=" << column->use_count() << ')';
} else {
if (column)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: statement should be inside braces [readability-braces-around-statements]

Suggested change
if (column)
if (column) {

be/src/vec/core/column_with_type_and_name.cpp:71:

-     else
+     } else

out << ' ' << column->dump_structure();
else
out << " nullptr";
Comment on lines +72 to 73
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: statement should be inside braces [readability-braces-around-statements]

Suggested change
else
out << " nullptr";
else {
out << " nullptr";
}

}
}

String ColumnWithTypeAndName::dump_structure() const {
Expand Down
Loading