Skip to content

Commit

Permalink
fix potential memcpy overlap issue in join (#8797) (#8801)
Browse files Browse the repository at this point in the history 
close #8796
  • Loading branch information
@ti-chi-bot
ti-chi-bot authored Feb 28, 2024
1 parent 0dd1cc9 commit 28a2314
Show file tree
Hide file tree
Showing 3 changed files with 81 additions and 31 deletions.
18 changes: 18 additions & 0 deletions dbms/src/Common/PODArray.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -634,8 +634,26 @@ class PODArray : public PODArrayBase<sizeof(T), INITIAL_SIZE, TAllocator, pad_ri
this->c_end = this->c_start + bytes_to_copy;
}

void assignFromSelf(size_t start, size_t end)
{
assert(end >= start && end <= this->size());
if unlikely (start == 0)
{
this->c_end = this->c_start + this->byte_size(end);
return;
}

size_t required_capacity = end - start;
size_t bytes_to_copy = this->byte_size(required_capacity);
const auto & from_begin = this->begin() + start;
memmove(this->c_start, reinterpret_cast<const void *>(&*from_begin), bytes_to_copy);
this->c_end = this->c_start + bytes_to_copy;
}

void assign(const PODArray & from)
{
if unlikely (this == &from)
return;
assign(from.begin(), from.end());
}

Expand Down
90 changes: 61 additions & 29 deletions dbms/src/Flash/tests/gtest_join_executor.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -561,36 +561,50 @@ CATCH
TEST_F(JoinExecutorTestRunner, Issue8791)
try
{
// clang-format off
auto build_key = toNullableVec<Int64>(
"id",
{
2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3,
4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5,
6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7,
2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2,
3, 3, 3, 3, 3, 3, 3, 3,
4, 4, 4, 4, 4,
5, 5, 5, 5, 5, 5,
6, 6, 6, 6, 6, 6,
7, 7, 7, 7, 7, 7,
});
auto build_col = toNullableVec<Int64>(
"build_value",
{
10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10,
10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10,
10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10,
10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10,
10, 30, 10, 30, 10, 30, 10, 10, 10, 30, 10, 30, 10, 30, 30,
10, 30, 10, 30, 10, 30, 10, 30,
30, 30, 30, 30, 30,
30, 30, 30, 30, 30, 10,
30, 30, 10, 30, 30, 30,
30, 30, 30, 10, 10, 30,
});
auto probe_key = toNullableVec<Int64>(
"id",
{
1, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3,
4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 6,
6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7,
1,
2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2,
3, 7, 3, 7, 3, 7, 3, 7, 3, 7, 3, 7, 3, 7, 3,
4, 6, 4, 6, 4, 6, 4, 6, 4, 6, 4, 6, 4, 6, 4,
5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5,
6, 4, 6, 4, 6, 4, 6, 4, 6, 4, 6, 4, 6, 4, 6,
7, 3, 7, 3, 7, 3, 7, 3, 7, 3, 7, 3, 7, 3, 7,
});
auto probe_col = toNullableVec<Int64>(
"probe_value",
{
20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20,
20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20,
20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20,
20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20,
20,
20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20,
20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20,
20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20,
20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20,
20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20,
20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20,
});
// clang-format on
context.addMockTable(
"issue_8791",
"build_table",
Expand All @@ -602,21 +616,39 @@ try
{{"id", TiDB::TP::TypeLongLong}, {"probe_value", TiDB::TP::TypeLongLong}},
{probe_key, probe_col});

auto request = context.scan("issue_8791", "probe_table")
.join(
context.scan("issue_8791", "build_table"),
tipb::JoinType::TypeAntiSemiJoin,
{col("id")},
{},
{},
{gt(col("probe_value"), col("build_value"))},
{})
.aggregation({Count(col("id"))}, {})
.build(context);

context.context->setSetting("max_block_size", Field(static_cast<UInt64>(200)));
auto expected_columns = {toVec<UInt64>({1})};
ASSERT_COLUMNS_EQ_UR(expected_columns, executeStreams(request, 1));
context.context->setSetting("max_block_size", Field(static_cast<UInt64>(90)));
{
auto anti_join_request = context.scan("issue_8791", "probe_table")
.join(
context.scan("issue_8791", "build_table"),
tipb::JoinType::TypeAntiSemiJoin,
{col("id")},
{},
{},
{gt(col("probe_value"), col("build_value"))},
{})
.aggregation({Count(col("id"))}, {})
.build(context);

auto expected_columns = {toVec<UInt64>({16})};
ASSERT_COLUMNS_EQ_UR(expected_columns, executeStreams(anti_join_request, 1));
}
{
auto inner_join_request = context.scan("issue_8791", "probe_table")
.join(
context.scan("issue_8791", "build_table"),
tipb::JoinType::TypeInnerJoin,
{col("id")},
{},
{},
{gt(col("probe_value"), col("build_value"))},
{})
.aggregation({Count(col("id"))}, {})
.build(context);

auto expected_columns = {toVec<UInt64>({240})};
ASSERT_COLUMNS_EQ_UR(expected_columns, executeStreams(inner_join_request, 1));
}
}
CATCH

Expand Down
4 changes: 2 additions & 2 deletions dbms/src/Interpreters/Join.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -875,9 +875,9 @@ Block Join::doJoinBlockHash(ProbeProcessInfo & probe_process_info) const
auto helper_col = block.getByName(match_helper_name).column;
helper_col = helper_col->cut(probe_process_info.start_row, probe_process_info.end_row);
}
offsets_to_replicate->assign(offsets_to_replicate->begin() + probe_process_info.start_row, offsets_to_replicate->begin() + probe_process_info.end_row);
offsets_to_replicate->assignFromSelf(probe_process_info.start_row, probe_process_info.end_row);
if (isAntiJoin(kind) && filter != nullptr)
filter->assign(filter->begin() + probe_process_info.start_row, filter->begin() + probe_process_info.end_row);
filter->assignFromSelf(probe_process_info.start_row, probe_process_info.end_row);
}
}
}
Expand Down

0 comments on commit 28a2314

Please sign in to comment.