Skip to content

Commit

Permalink
branch-3.0: [fix](delete) Fix static type dispatch by mistake due to …
Browse files Browse the repository at this point in the history
…typo (apache#43022)

PR Body: ## Proposed changes

DeletePredicatePB should be DeleteSubPredicatePB.

Test case is too ambiguous to add, since this bug is triggered by a huge
random test and failed to find the minimal case. However, this fix is
verified under the wild test that it does works.

Note that this problem may be triggered by another bug, cuz schema in
delete predicate rowset should contain column referred in delete
condition. Even if we don't have this fix, this error should never
happend.

But this error occurred under wild tests, means that schema in delete
predicate rowset is not adaptable with delete condition. I think it is
under some status that delete operation use BE tablet schema rather than
schema from FE, and the former rename operation result in that status.
But I failed to add a test case to reproduce, and think that by no way
will it happend occurding to the related code.
```
(1105, 'errCode = 2, detailMessage = ([172.20.50.7](http://172.20.50.7/))[INTERNAL_ERROR]failed to initialize storage reader. tablet=78026, res=[INTERNAL_ERROR]column not found, name=loc1, table_id=-1, schema_version=2

\t0#  doris::TabletSchema::column(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) const at /home/zcp/repo_center/doris_master/doris/be/src/common/status.h:375
\t1#  doris::Status doris::DeleteHandler::_parse_column_pred<doris::DeleteSubPredicatePB>(std::shared_ptr<doris::TabletSchema>, std::shared_ptr<doris::TabletSchema>, google::protobuf::RepeatedPtrField<doris::DeleteSubPredicatePB> const&, doris::DeleteConditions*) at /home/zcp/repo_center/doris_master/doris/be/src/util/expected.hpp:1986
\t2#  doris::DeleteHandler::init(std::shared_ptr<doris::TabletSchema>, std::vector<std::shared_ptr<doris::RowsetMeta>, std::allocator<std::shared_ptr<doris::RowsetMeta> > > const&, long) at /var/local/ldb-toolchain/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/shared_ptr_base.h:701
\t3#  doris::TabletReader::_init_delete_condition(doris::TabletReader::ReaderParams const&) at /var/local/ldb-toolchain/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/shared_ptr_base.h:701
\t4#  doris::TabletReader::_init_params(doris::TabletReader::ReaderParams const&) at /home/zcp/repo_center/doris_master/doris/be/src/common/status.h:499
\t5#  doris::TabletReader::init(doris::TabletReader::ReaderParams const&) at /home/zcp/repo_center/doris_master/doris/be/src/common/status.h:499
\t6#  doris::vectorized::BlockReader::init(doris::TabletReader::ReaderParams const&) at /home/zcp/repo_center/doris_master/doris/be/src/common/status.h:499
\t7#  doris::vectorized::NewOlapScanner::open(doris::RuntimeState*) at /home/zcp/repo_center/doris_master/doris/be/src/common/status.h:499
\t8#  doris::vectorized::ScannerScheduler::_scanner_scan(std::shared_ptr<doris::vectorized::ScannerContext>, std::shared_ptr<doris::vectorized::ScanTask>) at /home/zcp/repo_center/doris_master/doris/be/src/common/status.h:388
\t9#  std::_Function_handler<void (), doris::vectorized::ScannerScheduler::submit(std::shared_ptr<doris::vectorized::ScannerContext>, std::shared_ptr<doris::vectorized::ScanTask>)::$_1::operator()() const::{lambda()#1}>::_M_invoke(std::_Any_data const&) at /var/local/ldb-toolchain/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/shared_ptr_base.h:701
\t10# doris::ThreadPool::dispatch_thread() at /home/zcp/repo_center/doris_master/doris/be/src/util/threadpool.cpp:0
\t11# doris::Thread::supervise_thread(void*) at /var/local/ldb-toolchain/bin/../usr/include/pthread.h:562
\t12# ?
\t13# ?
, backend=[172.20.50.7](http://172.20.50.7/)')
```

```cpp
auto tablet_schema = std::make_shared<TabletSchema>();
    tablet_schema->copy_from(*tablet->tablet_schema());
    if (!request.columns_desc.empty() && request.columns_desc[0].col_unique_id >= 0) {
        tablet_schema->clear_columns();
        // TODO(lhy) handle variant
        for (const auto& column_desc : request.columns_desc) {
            tablet_schema->append_column(TabletColumn(column_desc));
        }
    }
    RowsetSharedPtr rowset_to_add;
    // writes
    res = _convert_v2(tablet, &rowset_to_add, tablet_schema, push_type);
    if (!res.ok()) {
        LOG(WARNING) << "fail to convert tmp file when realtime push. res=" << res
                     << ", failed to process realtime push."
                     << ", tablet=" << tablet->tablet_id()
                     << ", transaction_id=" << request.transaction_id;

        Status rollback_status = _engine.txn_manager()->rollback_txn(request.partition_id, *tablet,
                                                                     request.transaction_id);
        // has to check rollback status to ensure not delete a committed rowset
        if (rollback_status.ok()) {
            _engine.add_unused_rowset(rowset_to_add);
        }
        return res;
    }

    // add pending data to tablet

    if (push_type == PushType::PUSH_FOR_DELETE) {
        rowset_to_add->rowset_meta()->set_delete_predicate(std::move(del_preds.front()));
        del_preds.pop();
    }
``` 
 Cherry-picked from apache#42260

Co-authored-by: Siyang Tang <82279870+TangSiyang2001@users.noreply.github.com>
  • Loading branch information
github-actions[bot] and TangSiyang2001 authored Nov 8, 2024
1 parent 35071eb commit 3fb9994
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 4 deletions.
13 changes: 9 additions & 4 deletions be/src/olap/delete_handler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -346,17 +346,22 @@ Status DeleteHandler::parse_condition(const std::string& condition_str, TConditi
}

template <typename SubPredType>
requires(std::is_same_v<SubPredType, DeleteSubPredicatePB> or
std::is_same_v<SubPredType, std::string>)
Status DeleteHandler::_parse_column_pred(TabletSchemaSPtr complete_schema,
TabletSchemaSPtr delete_pred_related_schema,
const RepeatedPtrField<SubPredType>& sub_pred_list,
DeleteConditions* delete_conditions) {
for (const auto& sub_predicate : sub_pred_list) {
TCondition condition;
RETURN_IF_ERROR(parse_condition(sub_predicate, &condition));
int32_t col_unique_id;
if constexpr (std::is_same_v<SubPredType, DeletePredicatePB>) {
col_unique_id = sub_predicate.col_unique_id;
} else {
int32_t col_unique_id = -1;
if constexpr (std::is_same_v<SubPredType, DeleteSubPredicatePB>) {
if (sub_predicate.has_column_unique_id()) [[likely]] {
col_unique_id = sub_predicate.column_unique_id();
}
}
if (col_unique_id < 0) {
const auto& column =
*DORIS_TRY(delete_pred_related_schema->column(condition.column_name));
col_unique_id = column.unique_id();
Expand Down
3 changes: 3 additions & 0 deletions be/src/olap/delete_handler.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

#include <cstdint>
#include <string>
#include <type_traits>

#include "common/factory_creator.h"
#include "common/status.h"
Expand Down Expand Up @@ -115,6 +116,8 @@ class DeleteHandler {

private:
template <typename SubPredType>
requires(std::is_same_v<SubPredType, DeleteSubPredicatePB> or
std::is_same_v<SubPredType, std::string>)
Status _parse_column_pred(
TabletSchemaSPtr complete_schema, TabletSchemaSPtr delete_pred_related_schema,
const ::google::protobuf::RepeatedPtrField<SubPredType>& sub_pred_list,
Expand Down

0 comments on commit 3fb9994

Please sign in to comment.