From e5b6826de6bd89cc1e909d4d6b74078632b25df8 Mon Sep 17 00:00:00 2001 From: zhannngchen <48427519+zhannngchen@users.noreply.github.com> Date: Thu, 28 Dec 2023 10:33:03 +0800 Subject: [PATCH] [fix](partial update) update error code when failed to fill the missing fields (#29103) 1. InternalError is not clear for such error, use InvalidSchema Error instead 2. avoid some useless stacktrace on InternalError when load failed --- be/src/olap/memtable_flush_executor.cpp | 3 ++- .../olap/rowset/segment_v2/segment_writer.cpp | 15 +++++++++++--- .../segment_v2/vertical_segment_writer.cpp | 15 +++++++++++--- be/src/vec/sink/writer/vtablet_writer.cpp | 20 ++++++++++--------- .../insert_into_table/partial_update.groovy | 2 +- ...t_partial_update_native_insert_stmt.groovy | 2 +- 6 files changed, 39 insertions(+), 18 deletions(-) diff --git a/be/src/olap/memtable_flush_executor.cpp b/be/src/olap/memtable_flush_executor.cpp index e02048b4c88e8c..ff2d16bd1f508f 100644 --- a/be/src/olap/memtable_flush_executor.cpp +++ b/be/src/olap/memtable_flush_executor.cpp @@ -161,7 +161,8 @@ void FlushToken::_flush_memtable(MemTable* mem_table, int32_t segment_id, } if (!s.ok()) { std::lock_guard wrlk(_flush_status_lock); - LOG(WARNING) << "Flush memtable failed with res = " << s; + LOG(WARNING) << "Flush memtable failed with res = " << s + << ", load_id: " << print_id(_rowset_writer->load_id()); _flush_status = s; return; } diff --git a/be/src/olap/rowset/segment_v2/segment_writer.cpp b/be/src/olap/rowset/segment_v2/segment_writer.cpp index a36b83f23b9f1d..e2fd6d5f503ee4 100644 --- a/be/src/olap/rowset/segment_v2/segment_writer.cpp +++ b/be/src/olap/rowset/segment_v2/segment_writer.cpp @@ -471,9 +471,18 @@ Status SegmentWriter::append_block_with_partial_content(const vectorized::Block* } if (!_opts.rowset_ctx->partial_update_info->can_insert_new_rows_in_partial_update) { - return Status::InternalError( - "the unmentioned columns should have default value or be nullable for " - "newly inserted rows in non-strict mode partial update"); + std::string error_column; + for (auto cid : _opts.rowset_ctx->partial_update_info->missing_cids) { + const TabletColumn& col = _tablet_schema->column(cid); + if (!col.has_default_value() && !col.is_nullable()) { + error_column = col.name(); + break; + } + } + return Status::Error( + "the unmentioned column `{}` should have default value or be nullable for " + "newly inserted rows in non-strict mode partial update", + error_column); } has_default_or_nullable = true; use_default_or_null_flag.emplace_back(true); diff --git a/be/src/olap/rowset/segment_v2/vertical_segment_writer.cpp b/be/src/olap/rowset/segment_v2/vertical_segment_writer.cpp index bc45092442eded..fe3204e242e436 100644 --- a/be/src/olap/rowset/segment_v2/vertical_segment_writer.cpp +++ b/be/src/olap/rowset/segment_v2/vertical_segment_writer.cpp @@ -405,9 +405,18 @@ Status VerticalSegmentWriter::_append_block_with_partial_content(RowsInBlock& da } if (!_opts.rowset_ctx->partial_update_info->can_insert_new_rows_in_partial_update) { - return Status::InternalError( - "the unmentioned columns should have default value or be nullable for " - "newly inserted rows in non-strict mode partial update"); + std::string error_column; + for (auto cid : _opts.rowset_ctx->partial_update_info->missing_cids) { + const TabletColumn& col = _tablet_schema->column(cid); + if (!col.has_default_value() && !col.is_nullable()) { + error_column = col.name(); + break; + } + } + return Status::Error( + "the unmentioned column `{}` should have default value or be nullable for " + "newly inserted rows in non-strict mode partial update", + error_column); } has_default_or_nullable = true; use_default_or_null_flag.emplace_back(true); diff --git a/be/src/vec/sink/writer/vtablet_writer.cpp b/be/src/vec/sink/writer/vtablet_writer.cpp index a6615c2592dbe7..3093dbc5f6b632 100644 --- a/be/src/vec/sink/writer/vtablet_writer.cpp +++ b/be/src/vec/sink/writer/vtablet_writer.cpp @@ -176,16 +176,16 @@ void IndexChannel::mark_as_failed(const VNodeChannel* node_channel, const std::s _failed_channels_msgs.emplace(the_tablet_id, err + ", host: " + node_channel->host()); if (_failed_channels[the_tablet_id].size() >= ((_parent->_num_replicas + 1) / 2)) { - _intolerable_failure_status = - Status::InternalError(_failed_channels_msgs[the_tablet_id]); + _intolerable_failure_status = Status::Error( + _failed_channels_msgs[the_tablet_id]); } } } else { _failed_channels[tablet_id].insert(node_id); _failed_channels_msgs.emplace(tablet_id, err + ", host: " + node_channel->host()); if (_failed_channels[tablet_id].size() >= ((_parent->_num_replicas + 1) / 2)) { - _intolerable_failure_status = - Status::InternalError(_failed_channels_msgs[tablet_id]); + _intolerable_failure_status = Status::Error( + _failed_channels_msgs[tablet_id]); } } } @@ -432,7 +432,7 @@ Status VNodeChannel::open_wait() { _cancelled = true; auto error_code = open_callback->cntl_->ErrorCode(); auto error_text = open_callback->cntl_->ErrorText(); - return Status::InternalError( + return Status::Error( "failed to open tablet writer, error={}, error_text={}, info={}", berror(error_code), error_text, channel_info()); } @@ -466,7 +466,8 @@ Status VNodeChannel::add_block(vectorized::Block* block, const Payload* payload, if (!st.ok()) { if (_cancelled) { std::lock_guard l(_cancel_msg_lock); - return Status::InternalError("add row failed. {}", _cancel_msg); + return Status::Error("add row failed. {}", + _cancel_msg); } else { return std::move(st.prepend("already stopped, can't add row. cancelled/eos: ")); } @@ -872,7 +873,8 @@ Status VNodeChannel::close_wait(RuntimeState* state) { if (!st.ok()) { if (_cancelled) { std::lock_guard l(_cancel_msg_lock); - return Status::InternalError("wait close failed. {}", _cancel_msg); + return Status::Error("wait close failed. {}", + _cancel_msg); } else { return std::move( st.prepend("already stopped, skip waiting for close. cancelled/!eos: ")); @@ -902,7 +904,7 @@ Status VNodeChannel::close_wait(RuntimeState* state) { return Status::OK(); } - return Status::InternalError(get_cancel_msg()); + return Status::Error(get_cancel_msg()); } void VNodeChannel::_close_check() { @@ -1049,7 +1051,7 @@ Status VTabletWriter::open(doris::RuntimeState* state, doris::RuntimeProfile* pr // start to send batch continually. this must be called after _init if (bthread_start_background(&_sender_thread, nullptr, periodic_send_batch, (void*)this) != 0) { - return Status::Error("bthread_start_backgroud failed"); + return Status::Error("bthread_start_backgroud failed"); } return Status::OK(); } diff --git a/regression-test/suites/nereids_p0/insert_into_table/partial_update.groovy b/regression-test/suites/nereids_p0/insert_into_table/partial_update.groovy index 5fcf4e63deeb5c..da697fa20e6083 100644 --- a/regression-test/suites/nereids_p0/insert_into_table/partial_update.groovy +++ b/regression-test/suites/nereids_p0/insert_into_table/partial_update.groovy @@ -122,7 +122,7 @@ suite("nereids_partial_update_native_insert_stmt", "p0") { // but field `name` is not nullable and doesn't have default value test { sql """insert into ${tableName3}(id,score) values(2,400),(1,200),(4,400)""" - exception "INTERNAL_ERROR" + exception "the unmentioned column `name` should have default value or be nullable" } sql "set enable_unique_key_partial_update=false;" sql "sync;" diff --git a/regression-test/suites/unique_with_mow_p0/partial_update/test_partial_update_native_insert_stmt.groovy b/regression-test/suites/unique_with_mow_p0/partial_update/test_partial_update_native_insert_stmt.groovy index 7cd9916527234c..bf128ba26b4861 100644 --- a/regression-test/suites/unique_with_mow_p0/partial_update/test_partial_update_native_insert_stmt.groovy +++ b/regression-test/suites/unique_with_mow_p0/partial_update/test_partial_update_native_insert_stmt.groovy @@ -122,7 +122,7 @@ suite("test_partial_update_native_insert_stmt", "p0") { // but field `name` is not nullable and doesn't have default value test { sql """insert into ${tableName3}(id,score) values(2,400),(1,200),(4,400)""" - exception "INTERNAL_ERROR" + exception "the unmentioned column `name` should have default value or be nullable" } sql "set enable_unique_key_partial_update=false;" sql "sync;"