From 88c1579fb8345ba89afd50afa16a892208cb447f Mon Sep 17 00:00:00 2001 From: obdev Date: Fri, 15 Dec 2023 07:17:23 +0000 Subject: [PATCH] bugfix : obkv bugfix patch master --- src/libtable/test/ob_batch_execute_test.cpp | 12 ++++----- .../table/ob_htable_filter_parser.cpp | 8 ++---- src/observer/table/ob_table_context.cpp | 21 ++++++++++------ src/observer/table/ob_table_filter.cpp | 4 +-- .../table/ob_table_insert_up_executor.cpp | 3 +++ .../table/ob_table_modify_executor.cpp | 25 +++++++++++++++++++ src/observer/table/ob_table_modify_executor.h | 2 ++ .../table/ttl/ob_table_ttl_executor.cpp | 3 +++ unittest/observer/table/hfilter_parser.test | 6 ++--- 9 files changed, 59 insertions(+), 25 deletions(-) diff --git a/src/libtable/test/ob_batch_execute_test.cpp b/src/libtable/test/ob_batch_execute_test.cpp index e47669c465..010d08a8bc 100644 --- a/src/libtable/test/ob_batch_execute_test.cpp +++ b/src/libtable/test/ob_batch_execute_test.cpp @@ -3500,7 +3500,7 @@ TEST_F(TestBatchExecute, update_table_with_index_by_lowercase_rowkey) ASSERT_EQ(OB_SUCCESS, update_entity->add_rowkey_value(rk_obj)); ASSERT_EQ(OB_SUCCESS, update_entity->set_property(T, v_obj)); ObTableOperation table_operation = ObTableOperation::insert_or_update(*update_entity); - ASSERT_EQ(OB_SUCCESS, the_table->execute(table_operation, r)); + ASSERT_EQ(OB_ERR_UPDATE_ROWKEY_COLUMN, the_table->execute(table_operation, r)); ASSERT_EQ(OB_SUCCESS, r.get_errno()); } { @@ -3522,7 +3522,7 @@ TEST_F(TestBatchExecute, update_table_with_index_by_lowercase_rowkey) ASSERT_EQ(1, result_entity->get_properties_count()); ASSERT_EQ(OB_SUCCESS, result_entity->get_property(K, obj1)); ASSERT_EQ(OB_SUCCESS, obj1.get_varchar(str)); - ASSERT_TRUE(str == ObString::make_string("test")); + ASSERT_TRUE(str == ObString::make_string("TEST")); } ASSERT_EQ(OB_ITER_END, iter->get_next_entity(result_entity)); } @@ -4692,9 +4692,9 @@ TEST_F(TestBatchExecute, single_insert_up) value.set_double(c2_value); ASSERT_EQ(OB_SUCCESS, entity->set_property(C2, value)); ObTableOperation table_operation = ObTableOperation::insert_or_update(*entity); - ASSERT_EQ(OB_SUCCESS, the_table->execute(table_operation, r)); + ASSERT_EQ(OB_ERR_UPDATE_ROWKEY_COLUMN, the_table->execute(table_operation, r)); ASSERT_EQ(OB_SUCCESS, r.get_errno()); - ASSERT_EQ(1, r.get_affected_rows()); + ASSERT_EQ(0, r.get_affected_rows()); ASSERT_EQ(ObTableOperationType::INSERT_OR_UPDATE, r.type()); ASSERT_EQ(OB_SUCCESS, r.get_entity(result_entity)); ASSERT_TRUE(result_entity->is_empty()); @@ -4737,9 +4737,9 @@ TEST_F(TestBatchExecute, single_insert_up) ASSERT_EQ(OB_SUCCESS, entity->set_property(C2, value)); ObTableOperation table_operation = ObTableOperation::insert_or_update(*entity); // TODO:@linjing 和sql行为不一致 - ASSERT_EQ(OB_SUCCESS, the_table->execute(table_operation, r)); + ASSERT_EQ(OB_ERR_UPDATE_ROWKEY_COLUMN, the_table->execute(table_operation, r)); ASSERT_EQ(OB_SUCCESS, r.get_errno()); - ASSERT_EQ(1, r.get_affected_rows()); + ASSERT_EQ(0, r.get_affected_rows()); ASSERT_EQ(ObTableOperationType::INSERT_OR_UPDATE, r.type()); ASSERT_EQ(OB_SUCCESS, r.get_entity(result_entity)); ASSERT_TRUE(result_entity->is_empty()); diff --git a/src/observer/table/ob_htable_filter_parser.cpp b/src/observer/table/ob_htable_filter_parser.cpp index 01477486c2..47b4386f5d 100644 --- a/src/observer/table/ob_htable_filter_parser.cpp +++ b/src/observer/table/ob_htable_filter_parser.cpp @@ -88,13 +88,9 @@ int ObHTableFilterParser::parse_filter(const ObString &filter_string, hfilter::F filter = result_filter_; } else { if (1 == ret) { // syntax error - if (OB_SUCCESS != error_code_) { - ret = error_code_; - } else { - ret = OB_ERR_PARSER_SYNTAX; - } + ret = OB_ERR_PARSER_SYNTAX; } - LOG_WARN("failed to parse filter", K(ret), K_(error_msg), K(filter_string)); + LOG_WARN("failed to parse filter", K(ret), K_(error_msg), K(filter_string), K_(error_code)); } ob_hfilter__delete_buffer(bp, scanner_); return ret; diff --git a/src/observer/table/ob_table_context.cpp b/src/observer/table/ob_table_context.cpp index 2667b4591a..abbd0f22d1 100644 --- a/src/observer/table/ob_table_context.cpp +++ b/src/observer/table/ob_table_context.cpp @@ -1047,14 +1047,19 @@ int ObTableCtx::init_assignments(const ObTableEntity &entity) for (int64_t i = 0; OB_SUCC(ret) && i < column_items_.count(); i++) { ObTableColumnItem &item = column_items_.at(i); if (OB_SUCCESS == entity.get_property(item.column_name_, prop_obj)) { - ObTableAssignment assign(&item); - assign.assign_value_ = prop_obj; // shadow copy when prop_obj is string type - assign.is_assigned_ = true; - if (OB_FAIL(assigns_.push_back(assign))) { - LOG_WARN("fail to push back assignment", K(ret), K_(assigns), K(assign)); - } else if (table_schema_->has_generated_column() - && OB_FAIL(add_stored_generated_column_assignment(assign))) { - LOG_WARN("fail to add soterd generated column assignment", K(ret), K(assign)); + if (item.rowkey_position_ > 0) { + ret = OB_ERR_UPDATE_ROWKEY_COLUMN; + LOG_WARN("can not update rowkey column", K(ret)); + } else { + ObTableAssignment assign(&item); + assign.assign_value_ = prop_obj; // shadow copy when prop_obj is string type + assign.is_assigned_ = true; + if (OB_FAIL(assigns_.push_back(assign))) { + LOG_WARN("fail to push back assignment", K(ret), K_(assigns), K(assign)); + } else if (table_schema_->has_generated_column() + && OB_FAIL(add_stored_generated_column_assignment(assign))) { + LOG_WARN("fail to add soterd generated column assignment", K(ret), K(assign)); + } } } else if (item.auto_filled_timestamp_) { // on update current timestamp ObTableAssignment assign(&item); diff --git a/src/observer/table/ob_table_filter.cpp b/src/observer/table/ob_table_filter.cpp index b675733a9b..b80634af30 100644 --- a/src/observer/table/ob_table_filter.cpp +++ b/src/observer/table/ob_table_filter.cpp @@ -296,7 +296,7 @@ int ObNormalTableQueryResultIterator::get_normal_result(table::ObTableQueryResul while (OB_SUCC(ret) && OB_SUCC(scan_result_->get_next_row(row))) { LOG_DEBUG("[yzfdebug] scan result", "row", *row); if (OB_FAIL(one_result_->add_row(*row))) { - if (OB_SIZE_OVERFLOW == ret) { + if (OB_BUF_NOT_ENOUGH == ret) { ret = OB_SUCCESS; last_row_ = row; break; @@ -491,7 +491,7 @@ int ObTableFilterOperator::get_normal_result(table::ObTableQueryResult *&next_re if (has_limit && row_idx_ < offset) { row_idx_++; } else if (OB_FAIL(one_result_->add_row(*row))) { - if (OB_SIZE_OVERFLOW == ret) { + if (OB_BUF_NOT_ENOUGH == ret) { ret = OB_SUCCESS; last_row_ = row; break; diff --git a/src/observer/table/ob_table_insert_up_executor.cpp b/src/observer/table/ob_table_insert_up_executor.cpp index dba11c3664..a697b0f0a1 100644 --- a/src/observer/table/ob_table_insert_up_executor.cpp +++ b/src/observer/table/ob_table_insert_up_executor.cpp @@ -212,6 +212,9 @@ int ObTableApiInsertUpExecutor::do_insert_up_cache() if (OB_ISNULL(upd_old_row)) { ret = OB_ERR_UNEXPECTED; LOG_WARN("upd_old_row is NULL", K(ret)); + } else if (OB_FAIL(check_rowkey_change(*upd_old_row, + *upd_new_row))) { + LOG_WARN("can not update rowkey column", K(ret)); } else if (OB_FAIL(check_whether_row_change(*upd_old_row, *upd_new_row, insert_up_spec_.get_ctdef().upd_ctdef_, diff --git a/src/observer/table/ob_table_modify_executor.cpp b/src/observer/table/ob_table_modify_executor.cpp index 7777c65448..f6e4a35261 100644 --- a/src/observer/table/ob_table_modify_executor.cpp +++ b/src/observer/table/ob_table_modify_executor.cpp @@ -410,6 +410,31 @@ int ObTableApiModifyExecutor::check_whether_row_change(const ObChunkDatumStore:: return ret; } +// if common column equal, check rowkey column, if not equal then report error +int ObTableApiModifyExecutor::check_rowkey_change(const ObChunkDatumStore::StoredRow &upd_old_row, + const ObChunkDatumStore::StoredRow &upd_new_row) +{ + int ret = OB_SUCCESS; + if (lib::is_mysql_mode()) { + if (OB_UNLIKELY(upd_old_row.cnt_ != upd_new_row.cnt_) + || OB_UNLIKELY(upd_old_row.cnt_ != tb_ctx_.get_column_items().count())) { + ret = OB_ERR_UNEXPECTED; + LOG_WARN("check column size failed", K(ret), K(upd_old_row.cnt_), + K(upd_new_row.cnt_), K(tb_ctx_.get_column_items().count())); + } + for (int64_t i = 0; OB_SUCC(ret) && i < tb_ctx_.get_column_items().count(); i++) { + ObTableColumnItem &item = tb_ctx_.get_column_items().at(i); + if (item.rowkey_position_ <= 0) { + // do nothing + } else if (!ObDatum::binary_equal(upd_old_row.cells()[i], upd_new_row.cells()[i])) { + ret = OB_ERR_UPDATE_ROWKEY_COLUMN; + LOG_WARN("can not update rowkey column", K(ret)); + } + } + } + return ret; +} + int ObTableApiModifyExecutor::to_expr_skip_old(const ObChunkDatumStore::StoredRow &store_row, const ObTableUpdCtDef &upd_ctdef) { diff --git a/src/observer/table/ob_table_modify_executor.h b/src/observer/table/ob_table_modify_executor.h index cc510f9eca..8ed83aa14f 100644 --- a/src/observer/table/ob_table_modify_executor.h +++ b/src/observer/table/ob_table_modify_executor.h @@ -107,6 +107,8 @@ protected: const ObChunkDatumStore::StoredRow &upd_new_row, const ObTableUpdCtDef &upd_ctdef, bool &is_row_changed); + int check_rowkey_change(const ObChunkDatumStore::StoredRow &upd_old_row, + const ObChunkDatumStore::StoredRow &upd_new_row); int to_expr_skip_old(const ObChunkDatumStore::StoredRow &store_row, const ObTableUpdCtDef &upd_ctdef); int generate_del_rtdef_for_update(const ObTableUpdCtDef &upd_ctdef, diff --git a/src/observer/table/ttl/ob_table_ttl_executor.cpp b/src/observer/table/ttl/ob_table_ttl_executor.cpp index 8a2a070580..ed2eb5fe65 100644 --- a/src/observer/table/ttl/ob_table_ttl_executor.cpp +++ b/src/observer/table/ttl/ob_table_ttl_executor.cpp @@ -210,6 +210,9 @@ int ObTableApiTTLExecutor::update_row_to_conflict_checker() if (OB_ISNULL(upd_old_row)) { ret = OB_ERR_UNEXPECTED; LOG_WARN("upd_old_row is NULL", K(ret)); + } else if (OB_FAIL(check_rowkey_change(*upd_old_row, + *upd_new_row))) { + LOG_WARN("can not update rowkey column", K(ret)); } else if (OB_FAIL(check_whether_row_change(*upd_old_row, *upd_new_row, ttl_spec_.get_ctdef().upd_ctdef_, diff --git a/unittest/observer/table/hfilter_parser.test b/unittest/observer/table/hfilter_parser.test index 87a5a2c38d..6c467f2eb3 100644 --- a/unittest/observer/table/hfilter_parser.test +++ b/unittest/observer/table/hfilter_parser.test @@ -20,15 +20,15 @@ RowF(<, 'binary:abc') RowFilter(<, 'binary --error 5006 RowFilter(<, 'binary'') ---error 4002 +--error 5006 RowFilter(<, 'binary''') --error 5006 RowFilter(<, 'binary:abc')) --error 5006 (RowFilter(<, 'binary:abc') ---error 4002 +--error 5006 RowFilter(>, 'substring:abc') ---error 4007 +--error 5006 RowFilter(>=, 'regexstring:abc') --error 5006 ( singlecolumnvaluefilter ( !=, 'substring:abc', 'cf1', 'c1') )