From 989a8f3547656344facb340ff68c002d84dce4bc Mon Sep 17 00:00:00 2001 From: chinaxing Date: Thu, 12 Oct 2023 08:40:05 +0000 Subject: [PATCH] fix set transaction characters --- .../engine/cmd/ob_variable_set_executor.cpp | 5 ++- src/sql/ob_result_set.cpp | 17 +++++--- src/sql/ob_sql_trans_control.cpp | 20 +++++---- src/sql/ob_sql_trans_control.h | 10 +++-- .../resolver/tcl/ob_start_trans_resolver.cpp | 2 + src/sql/session/ob_basic_session_info.cpp | 43 +++++++++++++------ src/sql/session/ob_basic_session_info.h | 7 ++- src/sql/session/ob_sql_session_info.cpp | 4 +- src/sql/session/ob_sql_session_info.h | 2 +- 9 files changed, 74 insertions(+), 36 deletions(-) diff --git a/src/sql/engine/cmd/ob_variable_set_executor.cpp b/src/sql/engine/cmd/ob_variable_set_executor.cpp index 659fe24c44..8e6e253786 100644 --- a/src/sql/engine/cmd/ob_variable_set_executor.cpp +++ b/src/sql/engine/cmd/ob_variable_set_executor.cpp @@ -1026,7 +1026,10 @@ int ObVariableSetExecutor::process_session_autocommit_hook(ObExecContext &exec_c } // skip commit txn if this is txn free route temporary node } else if (false == orig_ac && true == in_trans && 1 == autocommit && !my_session->is_txn_free_route_temp()) { - if (OB_FAIL(ObSqlTransControl::implicit_end_trans(exec_ctx, false))) { + // set autocommit = 1 won't clear next scope transaction settings: + // `set transaction read only` + // `set transaction isolation level` + if (OB_FAIL(ObSqlTransControl::implicit_end_trans(exec_ctx, false, NULL, false))) { LOG_WARN("fail implicit commit trans", K(ret)); } } else { diff --git a/src/sql/ob_result_set.cpp b/src/sql/ob_result_set.cpp index 2ddc23f05d..465ee4106e 100644 --- a/src/sql/ob_result_set.cpp +++ b/src/sql/ob_result_set.cpp @@ -277,9 +277,16 @@ int ObResultSet::on_cmd_execute() } get_exec_context().set_need_disconnect(false); } else { - // commit current open transaction, synchronously - if (OB_FAIL(ObSqlTransControl::implicit_end_trans(get_exec_context(), false))) { - SQL_ENG_LOG(WARN, "fail end implicit trans on cmd execute", K(ret)); + // implicit end transaction and start transaction will not clear next scope transaction settings by: + // a. set by `set transaction read only` + // b. set by `set transaction isolation level XXX` + const int cmd_type = cmd_->get_cmd_type(); + bool keep_trans_variable = (cmd_type == stmt::T_START_TRANS); + if (OB_FAIL(ObSqlTransControl::implicit_end_trans(get_exec_context(), false, NULL, !keep_trans_variable))) { + LOG_WARN("fail end implicit trans on cmd execute", K(ret)); + } else if (my_session_.need_recheck_txn_readonly() && my_session_.get_tx_read_only()) { + ret = OB_ERR_CANT_EXECUTE_IN_READ_ONLY_TRANSACTION; + LOG_WARN("cmd can not execute because txn is read only", K(ret), K(cmd_type)); } } } @@ -967,7 +974,7 @@ OB_INLINE int ObResultSet::auto_end_plan_trans(ObPhysicalPlan& plan, K(in_trans), K(ac), K(explicit_trans), K(is_async_end_trans_submitted())); // explicit start trans will disable auto-commit - if (!explicit_trans && ac) { + if (!explicit_trans && ac && plan.is_need_trans()) { ObPhysicalPlanCtx *plan_ctx = NULL; if (OB_ISNULL(plan_ctx = get_exec_context().get_physical_plan_ctx())) { ret = OB_ERR_UNEXPECTED; @@ -1027,7 +1034,7 @@ OB_INLINE int ObResultSet::auto_end_plan_trans(ObPhysicalPlan& plan, } NG_TRACE(auto_end_plan_end); LOG_TRACE("auto_end_plan_trans.end", K(ret), - K(in_trans), K(ac), K(explicit_trans), + K(in_trans), K(ac), K(explicit_trans), K(plan.is_need_trans()), K(is_rollback), K(async), K(is_async_end_trans_submitted())); return ret; diff --git a/src/sql/ob_sql_trans_control.cpp b/src/sql/ob_sql_trans_control.cpp index 120021a011..b44cc3b303 100644 --- a/src/sql/ob_sql_trans_control.cpp +++ b/src/sql/ob_sql_trans_control.cpp @@ -198,7 +198,8 @@ int ObSqlTransControl::explicit_start_trans(ObExecContext &ctx, const bool read_ int ObSqlTransControl::implicit_end_trans(ObExecContext &exec_ctx, const bool is_rollback, - ObEndTransAsyncCallback *callback) + ObEndTransAsyncCallback *callback, + bool reset_trans_variable) { int ret = OB_SUCCESS; #ifndef NDEBUG @@ -210,7 +211,7 @@ int ObSqlTransControl::implicit_end_trans(ObExecContext &exec_ctx, OX (tx_id = session->get_tx_id().get_id()); CHECK_TX_FREE_ROUTE(exec_ctx, session); FLTSpanGuard(end_transaction); - OZ(end_trans(exec_ctx, is_rollback, false, callback)); + OZ(end_trans(exec_ctx, is_rollback, false, callback, reset_trans_variable)); FLT_SET_TAG(trans_id, tx_id); return ret; } @@ -246,7 +247,8 @@ int ObSqlTransControl::explicit_end_trans(ObExecContext &exec_ctx, const bool is int ObSqlTransControl::end_trans(ObExecContext &exec_ctx, const bool is_rollback, const bool is_explicit, - ObEndTransAsyncCallback *callback) + ObEndTransAsyncCallback *callback, + bool reset_trans_variable) { int ret = OB_SUCCESS; bool sync = false; @@ -278,7 +280,7 @@ int ObSqlTransControl::end_trans(ObExecContext &exec_ctx, } callback->callback(OB_SUCCESS); } else { - reset_session_tx_state(session, true); + reset_session_tx_state(session, true, reset_trans_variable); exec_ctx.set_need_disconnect(false); } } else { @@ -301,7 +303,7 @@ int ObSqlTransControl::end_trans(ObExecContext &exec_ctx, bool reuse_tx = OB_SUCCESS == ret || OB_TRANS_COMMITED == ret || OB_TRANS_ROLLBACKED == ret; - reset_session_tx_state(session, reuse_tx); + reset_session_tx_state(session, reuse_tx, reset_trans_variable); } } if (callback && !is_rollback) { @@ -1188,7 +1190,7 @@ int ObSqlTransControl::get_trans_result(ObExecContext &exec_ctx) return get_trans_result(exec_ctx, exec_ctx.get_my_session()->get_trans_result()); } -int ObSqlTransControl::reset_session_tx_state(ObBasicSessionInfo *session, bool reuse_tx_desc) +int ObSqlTransControl::reset_session_tx_state(ObBasicSessionInfo *session, bool reuse_tx_desc, bool reset_trans_variable) { int ret = OB_SUCCESS; LOG_DEBUG("reset session tx state", KPC(session->get_tx_desc()), K(lbt())); @@ -1213,11 +1215,11 @@ int ObSqlTransControl::reset_session_tx_state(ObBasicSessionInfo *session, bool } } session->get_trans_result().reset(); - session->reset_tx_variable(); + session->reset_tx_variable(reset_trans_variable); return ret; } -int ObSqlTransControl::reset_session_tx_state(ObSQLSessionInfo *session, bool reuse_tx_desc) +int ObSqlTransControl::reset_session_tx_state(ObSQLSessionInfo *session, bool reuse_tx_desc, bool reset_trans_variable) { int temp_ret = OB_SUCCESS; // cleanup txn level temp tables if this is the txn start node @@ -1230,7 +1232,7 @@ int ObSqlTransControl::reset_session_tx_state(ObSQLSessionInfo *session, bool re LOG_WARN_RET(temp_ret, "trx level temporary table clean failed", KR(temp_ret)); } } - int ret = reset_session_tx_state(static_cast(session), reuse_tx_desc); + int ret = reset_session_tx_state(static_cast(session), reuse_tx_desc, reset_trans_variable); return COVER_SUCC(temp_ret); } diff --git a/src/sql/ob_sql_trans_control.h b/src/sql/ob_sql_trans_control.h index 68c41b6d3c..0a853c8b54 100644 --- a/src/sql/ob_sql_trans_control.h +++ b/src/sql/ob_sql_trans_control.h @@ -167,19 +167,21 @@ private: class ObSqlTransControl { public: - static int reset_session_tx_state(ObSQLSessionInfo *session, bool reuse_tx_desc = false); - static int reset_session_tx_state(ObBasicSessionInfo *session, bool reuse_tx_desc = false); + static int reset_session_tx_state(ObSQLSessionInfo *session, bool reuse_tx_desc = false, bool active_tx_end = true); + static int reset_session_tx_state(ObBasicSessionInfo *session, bool reuse_tx_desc = false, bool active_tx_end = true); static int create_stash_savepoint(ObExecContext &exec_ctx, const ObString &name); static int release_stash_savepoint(ObExecContext &exec_ctx, const ObString &name); static int explicit_start_trans(ObExecContext &exec_ctx, const bool read_only, const ObString hint = ObString()); static int explicit_end_trans(ObExecContext &exec_ctx, const bool is_rollback, const ObString hint = ObString()); static int implicit_end_trans(ObExecContext &exec_ctx, const bool is_rollback, - ObEndTransAsyncCallback *callback = NULL); + ObEndTransAsyncCallback *callback = NULL, + bool reset_trans_variable = true); static int end_trans(ObExecContext &exec_ctx, const bool is_rollback, const bool is_explicit, - ObEndTransAsyncCallback *callback = NULL); + ObEndTransAsyncCallback *callback = NULL, + bool reset_trans_variable = true); static int rollback_trans(ObSQLSessionInfo *session, bool &need_disconnect); static int do_end_trans_(ObSQLSessionInfo *session, diff --git a/src/sql/resolver/tcl/ob_start_trans_resolver.cpp b/src/sql/resolver/tcl/ob_start_trans_resolver.cpp index 2484fac2e8..c6ab27a6f2 100644 --- a/src/sql/resolver/tcl/ob_start_trans_resolver.cpp +++ b/src/sql/resolver/tcl/ob_start_trans_resolver.cpp @@ -68,6 +68,8 @@ int ObStartTransResolver::resolve(const ParseNode &parse_node) start_stmt->set_read_only(true); } else if (IS_READ_WRITE(parse_node.children_[0]->value_)) { start_stmt->set_read_only(false); + } else { + start_stmt->set_read_only(session_info_->get_tx_read_only()); } if (IS_WITH_SNAPSHOT(parse_node.children_[0]->value_)) { start_stmt->set_with_consistent_snapshot(true); diff --git a/src/sql/session/ob_basic_session_info.cpp b/src/sql/session/ob_basic_session_info.cpp index 45f6ded461..27210595db 100644 --- a/src/sql/session/ob_basic_session_info.cpp +++ b/src/sql/session/ob_basic_session_info.cpp @@ -142,6 +142,7 @@ ObBasicSessionInfo::ObBasicSessionInfo(const uint64_t tenant_id) is_tenant_killed_(0), reused_count_(0), first_need_txn_stmt_type_(stmt::T_NONE), + need_recheck_txn_readonly_(false), exec_min_cluster_version_(GET_MIN_CLUSTER_VERSION()), stmt_type_(stmt::T_NONE), labels_(), @@ -425,6 +426,7 @@ void ObBasicSessionInfo::reset(bool skip_sys_var) // 不要重置release_to_pool_，原因见属性声明位置的注释。 is_tenant_killed_ = 0; first_need_txn_stmt_type_ = stmt::T_NONE; + need_recheck_txn_readonly_ = false; exec_min_cluster_version_ = GET_MIN_CLUSTER_VERSION(); labels_.reuse(); thread_id_ = 0; @@ -5357,12 +5359,14 @@ int ObBasicSessionInfo::get_regexp_time_limit(int64_t &v) const return get_sys_variable(SYS_VAR_REGEXP_TIME_LIMIT, v); } -void ObBasicSessionInfo::reset_tx_variable() +void ObBasicSessionInfo::reset_tx_variable(bool reset_next_scope) { LOG_DEBUG("reset tx variable", K(lbt())); reset_first_need_txn_stmt_type(); - reset_tx_isolation(); - reset_tx_read_only(); + if (reset_next_scope) { + reset_tx_isolation(); + reset_tx_read_only(); + } reset_trans_flags(); clear_app_trace_id(); } @@ -5439,18 +5443,33 @@ void ObBasicSessionInfo::reset_tx_read_only() int ObBasicSessionInfo::check_tx_read_only_privilege(const ObSqlTraits &sql_traits) { int ret = OB_SUCCESS; - if (sql_traits.is_cause_implicit_commit_) { - reset_tx_read_only(); - } else {} - // pl call stmt and anonymous does not check here, - // will be check in ObTransService::start_stmt_sanity_check_ - if (get_tx_read_only() && !sql_traits.is_readonly_stmt_ + set_need_recheck_txn_readonly(false); + bool read_only = tx_desc_ && tx_desc_->is_in_tx() ? tx_desc_->is_rdonly() : get_tx_read_only(); + if (!sql_traits.is_readonly_stmt_ && sql_traits.stmt_type_ != ObItemType::T_SP_CALL_STMT && sql_traits.stmt_type_ != ObItemType::T_SP_ANONYMOUS_BLOCK && sql_traits.stmt_type_ != ObItemType::T_EXPLAIN) { - ret = OB_ERR_CANT_EXECUTE_IN_READ_ONLY_TRANSACTION; - - } else {} + if (sql_traits.is_cause_implicit_commit_ && !sql_traits.is_commit_stmt_) { + if (sys_vars_cache_.get_tx_read_only()) { + // should implicit commit current transaction before report error + // + // example case: + // + // set session transaction read only; + // set transaction read write; + // start transaction; + // insert into t values(1); + // create table t1(id int); -- error: OB_ERR_CANT_EXECUTE_IN_READ_ONLY_TRANSACTION + // rollback; + // select * from t where id = 1; -- 1 row found + // + set_need_recheck_txn_readonly(true); + } + } else if (read_only) { + ret = OB_ERR_CANT_EXECUTE_IN_READ_ONLY_TRANSACTION; + } + } + LOG_DEBUG("CHECK readonly", KP(this), K(sessid_), K(sql_traits.is_readonly_stmt_), K(get_tx_read_only())); return ret; } diff --git a/src/sql/session/ob_basic_session_info.h b/src/sql/session/ob_basic_session_info.h index d4c7768fe4..5d0a96c313 100644 --- a/src/sql/session/ob_basic_session_info.h +++ b/src/sql/session/ob_basic_session_info.h @@ -1124,7 +1124,7 @@ public: int set_init_connect(const common::ObString &init_sql); int save_trans_status(); // 重置事务相关变量 - virtual void reset_tx_variable(); + virtual void reset_tx_variable(bool reset_next_scope = true); transaction::ObTxIsolationLevel get_tx_isolation() const; bool is_isolation_serializable() const; void set_tx_isolation(transaction::ObTxIsolationLevel isolation); @@ -1285,6 +1285,8 @@ public: } inline void reset_first_need_txn_stmt_type() { first_need_txn_stmt_type_ = stmt::T_NONE; } inline stmt::StmtType get_first_need_txn_stmt_type() const { return first_need_txn_stmt_type_; } + inline void set_need_recheck_txn_readonly(bool need) { need_recheck_txn_readonly_ = need; } + inline bool need_recheck_txn_readonly() const { return need_recheck_txn_readonly_; } void set_stmt_type(stmt::StmtType stmt_type) { stmt_type_ = stmt_type; } stmt::StmtType get_stmt_type() const { return stmt_type_; } @@ -2179,7 +2181,8 @@ private: // type of first stmt which need transaction // either transactional read or transactional write stmt::StmtType first_need_txn_stmt_type_; - + // some Cmd like DDL will commit current transaction, and need recheck tx read only settings before run + bool need_recheck_txn_readonly_; //min_cluster_version_: 记录sql执行前当前集群最小的server版本号 //解决的问题兼容性问题: // 2.2.3之前的版本会序列化所有的需要序列化的系统变量, diff --git a/src/sql/session/ob_sql_session_info.cpp b/src/sql/session/ob_sql_session_info.cpp index 15f6c71138..1a102a24d8 100644 --- a/src/sql/session/ob_sql_session_info.cpp +++ b/src/sql/session/ob_sql_session_info.cpp @@ -2916,9 +2916,9 @@ void ObSQLSessionInfo::check_txn_free_route_alive() ObSqlTransControl::check_free_route_tx_alive(*this, txn_free_route_ctx_); } -void ObSQLSessionInfo::reset_tx_variable() +void ObSQLSessionInfo::reset_tx_variable(bool reset_next_scope) { - ObBasicSessionInfo::reset_tx_variable(); + ObBasicSessionInfo::reset_tx_variable(reset_next_scope); set_early_lock_release(false); } void ObSQLSessionInfo::destroy_contexts_map(ObContextsMap &map, common::ObIAllocator &alloc) diff --git a/src/sql/session/ob_sql_session_info.h b/src/sql/session/ob_sql_session_info.h index 3227d75f8a..29aa6c4a8d 100644 --- a/src/sql/session/ob_sql_session_info.h +++ b/src/sql/session/ob_sql_session_info.h @@ -1265,7 +1265,7 @@ public: uint64_t get_conn_res_user_id() const { return conn_res_user_id_; } int on_user_connect(share::schema::ObSessionPrivInfo &priv_info, const ObUserInfo *user_info); int on_user_disconnect(); - virtual void reset_tx_variable(); + virtual void reset_tx_variable(bool reset_next_scope = true); ObOptimizerTraceImpl& get_optimizer_tracer() { return optimizer_tracer_; } public: bool has_tx_level_temp_table() const { return tx_desc_ && tx_desc_->with_temporary_table(); }