From e4e714141d567c93f10c97af9b082e8e9e1d0b63 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 20 Nov 2025 17:08:48 +0000 Subject: [PATCH 01/10] Initial plan From 723e4b4d575fae717181a2dd5ad1e8c89c8d9794 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 20 Nov 2025 17:17:45 +0000 Subject: [PATCH 02/10] Migrate YDB transactions from Table Client to Query Client API - Updated Transaction class to use NYdb::NQuery::TSession and transaction ID - Changed Transaction::Execute() to use Query Client's ExecuteQuery - Modified Transaction::Commit() and Rollback() to use Query Client APIs - Updated TableClient::Begin() to use RetryQuery and Query Client BeginTransaction - Added YDB-CPP-SDK::Query library to CMakeLists.txt - Added conversion logic for QuerySettings between Table and Query Client APIs Co-authored-by: asmyasnikov <14202262+asmyasnikov@users.noreply.github.com> --- ydb/CMakeLists.txt | 1 + ydb/include/userver/ydb/transaction.hpp | 8 +++-- ydb/src/ydb/table.cpp | 10 +++--- ydb/src/ydb/transaction.cpp | 45 +++++++++++++++++++------ 4 files changed, 46 insertions(+), 18 deletions(-) diff --git a/ydb/CMakeLists.txt b/ydb/CMakeLists.txt index ee5d7cacd561..ce8244ca21dc 100644 --- a/ydb/CMakeLists.txt +++ b/ydb/CMakeLists.txt @@ -7,6 +7,7 @@ set(YDB_LIBRARIES YDB-CPP-SDK::Coordination YDB-CPP-SDK::Driver YDB-CPP-SDK::Operation + YDB-CPP-SDK::Query YDB-CPP-SDK::Result YDB-CPP-SDK::Scheme YDB-CPP-SDK::SolomonStats diff --git a/ydb/include/userver/ydb/transaction.hpp b/ydb/include/userver/ydb/transaction.hpp index bdf103f924bc..940b25151101 100644 --- a/ydb/include/userver/ydb/transaction.hpp +++ b/ydb/include/userver/ydb/transaction.hpp @@ -2,6 +2,8 @@ #include +#include + #include #include @@ -69,7 +71,8 @@ class Transaction final { // For internal use only. Transaction( TableClient& table_client, - NYdb::NTable::TTransaction ydb_tx, + NYdb::NQuery::TSession ydb_session, + std::string tx_id, std::string name, OperationSettings&& rollback_settings ) noexcept; @@ -85,7 +88,8 @@ class Transaction final { std::string name_; impl::StatsScope stats_scope_; tracing::Span span_; - NYdb::NTable::TTransaction ydb_tx_; + NYdb::NQuery::TSession ydb_session_; + std::string tx_id_; OperationSettings rollback_settings_; bool is_active_{true}; utils::trx_tracker::TransactionLock trx_lock_; diff --git a/ydb/src/ydb/table.cpp b/ydb/src/ydb/table.cpp index 6a7c41c3f22f..d443bc7ba7c6 100644 --- a/ydb/src/ydb/table.cpp +++ b/ydb/src/ydb/table.cpp @@ -311,20 +311,20 @@ Transaction TableClient::Begin(utils::StringLiteral transaction_name, OperationS Transaction TableClient::Begin(DynamicTransactionName transaction_name, OperationSettings settings) { const Query query{"", Query::Name{"Begin"}}; impl::RequestContext context{*this, query, std::move(settings)}; - auto tx_settings = PrepareTxSettings(context.settings); + auto tx_settings = PrepareQueryTxSettings(context.settings); - auto future = impl::RetryOperation( + auto future = impl::RetryQuery( context, [tx_settings = std::move(tx_settings), settings = context.settings, - deadline = context.deadline](NYdb::NTable::TSession session) { - const auto exec_settings = impl::PrepareRequestSettings(settings, deadline); + deadline = context.deadline](NYdb::NQuery::TSession session) { + const auto exec_settings = impl::PrepareRequestSettings(settings, deadline); return session.BeginTransaction(tx_settings, exec_settings); } ); auto status = impl::GetFutureValueChecked(std::move(future), "BeginTransaction", context); - return Transaction(*this, status.GetTransaction(), transaction_name.GetUnderlying(), std::move(settings)); + return Transaction(*this, status.GetTransaction().GetSession(), status.GetTransaction().GetId(), transaction_name.GetUnderlying(), std::move(settings)); } void TableClient::ExecuteSchemeQuery(const std::string& query) { diff --git a/ydb/src/ydb/transaction.cpp b/ydb/src/ydb/transaction.cpp index 3278db9b7cbe..8ead44ba41e9 100644 --- a/ydb/src/ydb/transaction.cpp +++ b/ydb/src/ydb/transaction.cpp @@ -20,7 +20,8 @@ namespace ydb { Transaction::Transaction( TableClient& table_client, - NYdb::NTable::TTransaction ydb_tx, + NYdb::NQuery::TSession ydb_session, + std::string tx_id, std::string name, OperationSettings&& rollback_settings ) noexcept @@ -28,7 +29,8 @@ Transaction::Transaction( name_(std::move(name)), stats_scope_(impl::StatsScope::TransactionTag{}, *table_client_.stats_, name_), span_("ydb_transaction"), - ydb_tx_(std::move(ydb_tx)), + ydb_session_(std::move(ydb_session)), + tx_id_(std::move(tx_id)), rollback_settings_(std::move(rollback_settings)) { span_.DetachFromCoroStack(); span_.AddTag("transaction_name", name_); @@ -78,7 +80,7 @@ void Transaction::Commit(OperationSettings settings) { if (data["trx_should_fail"].As()) { LOG_WARNING() << "Doing Rollback instead of commit " "due to Testpoint response"; - ydb_tx_.Rollback(); + ydb_session_.RollbackTransaction(tx_id_); throw TransactionForceRollback(); } } @@ -86,12 +88,12 @@ void Transaction::Commit(OperationSettings settings) { } const auto commit_settings = - impl::PrepareRequestSettings(context.settings, context.deadline); + impl::PrepareRequestSettings(context.settings, context.deadline); auto error_guard = ErrorGuard(); impl::GetFutureValueChecked( - ydb_tx_.Commit(commit_settings), "Commit", table_client_.driver_->GetRetryBudget(), context + ydb_session_.CommitTransaction(tx_id_, commit_settings), "Commit", table_client_.driver_->GetRetryBudget(), context ); error_guard.Release(); @@ -107,12 +109,12 @@ void Transaction::Rollback() { impl::RequestContext context{table_client_, kQuery, std::move(settings), impl::IsStreaming{false}, &span_}; const auto rollback_settings = - impl::PrepareRequestSettings(context.settings, context.deadline); + impl::PrepareRequestSettings(context.settings, context.deadline); [[maybe_unused]] auto error_guard = ErrorGuard(); impl::GetFutureValueChecked( - ydb_tx_.Rollback(rollback_settings), "Rollback", table_client_.driver_->GetRetryBudget(), context + ydb_session_.RollbackTransaction(tx_id_, rollback_settings), "Rollback", table_client_.driver_->GetRetryBudget(), context ); trx_lock_.Unlock(); @@ -121,7 +123,7 @@ void Transaction::Rollback() { } PreparedArgsBuilder Transaction::GetBuilder() const { - return PreparedArgsBuilder(ydb_tx_.GetSession().GetParamsBuilder()); + return PreparedArgsBuilder(ydb_session_.GetParamsBuilder()); } void Transaction::EnsureActive() const { @@ -145,16 +147,37 @@ ExecuteResponse Transaction::Execute( impl::RequestContext context{table_client_, query, std::move(settings), impl::IsStreaming{false}, &span_}; auto internal_params = std::move(builder).Build(); - auto exec_settings = table_client_.ToExecQuerySettings(query_settings); + // Convert QuerySettings to Query Client settings + NYdb::NQuery::TExecuteQuerySettings exec_settings; + if (query_settings.keep_in_query_cache.has_value()) { + // Query Client doesn't have KeepInQueryCache, it caches automatically + } + if (query_settings.collect_query_stats) { + // Convert Table Client stats mode to Query Client stats mode + switch (*query_settings.collect_query_stats) { + case NYdb::NTable::ECollectQueryStatsMode::None: + exec_settings.StatsMode(NYdb::NQuery::EStatsMode::None); + break; + case NYdb::NTable::ECollectQueryStatsMode::Basic: + exec_settings.StatsMode(NYdb::NQuery::EStatsMode::Basic); + break; + case NYdb::NTable::ECollectQueryStatsMode::Full: + exec_settings.StatsMode(NYdb::NQuery::EStatsMode::Full); + break; + case NYdb::NTable::ECollectQueryStatsMode::Profile: + exec_settings.StatsMode(NYdb::NQuery::EStatsMode::Profile); + break; + } + } impl::ApplyToRequestSettings(exec_settings, context.settings, context.deadline); // Must go after PrepareExecuteSettings, because an exception from there // leaves the transaction active. auto error_guard = ErrorGuard(); - auto execute_fut = ydb_tx_.GetSession().ExecuteDataQuery( + auto execute_fut = ydb_session_.ExecuteQuery( impl::ToString(query.GetStatementView()), - NYdb::NTable::TTxControl::Tx(ydb_tx_), + NYdb::NQuery::TTxControl::Tx(tx_id_), std::move(internal_params), exec_settings ); From d59561a2d4405e9b1921af42699ba87a424f7c7d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 20 Nov 2025 17:21:28 +0000 Subject: [PATCH 03/10] Refine transaction implementation to use NYdb::NQuery::TTransaction - Updated Transaction class to use NYdb::NQuery::TTransaction object - This matches the pattern of Table Client but uses Query namespace - Transaction object provides Commit(), Rollback(), and GetSession() methods - Execute uses TTxControl::Tx(transaction) for transaction control Co-authored-by: asmyasnikov <14202262+asmyasnikov@users.noreply.github.com> --- ydb/include/userver/ydb/transaction.hpp | 6 ++---- ydb/src/ydb/table.cpp | 2 +- ydb/src/ydb/transaction.cpp | 18 ++++++++---------- 3 files changed, 11 insertions(+), 15 deletions(-) diff --git a/ydb/include/userver/ydb/transaction.hpp b/ydb/include/userver/ydb/transaction.hpp index 940b25151101..f35b23c96c8e 100644 --- a/ydb/include/userver/ydb/transaction.hpp +++ b/ydb/include/userver/ydb/transaction.hpp @@ -71,8 +71,7 @@ class Transaction final { // For internal use only. Transaction( TableClient& table_client, - NYdb::NQuery::TSession ydb_session, - std::string tx_id, + NYdb::NQuery::TTransaction ydb_tx, std::string name, OperationSettings&& rollback_settings ) noexcept; @@ -88,8 +87,7 @@ class Transaction final { std::string name_; impl::StatsScope stats_scope_; tracing::Span span_; - NYdb::NQuery::TSession ydb_session_; - std::string tx_id_; + NYdb::NQuery::TTransaction ydb_tx_; OperationSettings rollback_settings_; bool is_active_{true}; utils::trx_tracker::TransactionLock trx_lock_; diff --git a/ydb/src/ydb/table.cpp b/ydb/src/ydb/table.cpp index d443bc7ba7c6..aca1b3ded06a 100644 --- a/ydb/src/ydb/table.cpp +++ b/ydb/src/ydb/table.cpp @@ -324,7 +324,7 @@ Transaction TableClient::Begin(DynamicTransactionName transaction_name, Operatio ); auto status = impl::GetFutureValueChecked(std::move(future), "BeginTransaction", context); - return Transaction(*this, status.GetTransaction().GetSession(), status.GetTransaction().GetId(), transaction_name.GetUnderlying(), std::move(settings)); + return Transaction(*this, status.GetTransaction(), transaction_name.GetUnderlying(), std::move(settings)); } void TableClient::ExecuteSchemeQuery(const std::string& query) { diff --git a/ydb/src/ydb/transaction.cpp b/ydb/src/ydb/transaction.cpp index 8ead44ba41e9..26b05c5c2a65 100644 --- a/ydb/src/ydb/transaction.cpp +++ b/ydb/src/ydb/transaction.cpp @@ -20,8 +20,7 @@ namespace ydb { Transaction::Transaction( TableClient& table_client, - NYdb::NQuery::TSession ydb_session, - std::string tx_id, + NYdb::NQuery::TTransaction ydb_tx, std::string name, OperationSettings&& rollback_settings ) noexcept @@ -29,8 +28,7 @@ Transaction::Transaction( name_(std::move(name)), stats_scope_(impl::StatsScope::TransactionTag{}, *table_client_.stats_, name_), span_("ydb_transaction"), - ydb_session_(std::move(ydb_session)), - tx_id_(std::move(tx_id)), + ydb_tx_(std::move(ydb_tx)), rollback_settings_(std::move(rollback_settings)) { span_.DetachFromCoroStack(); span_.AddTag("transaction_name", name_); @@ -80,7 +78,7 @@ void Transaction::Commit(OperationSettings settings) { if (data["trx_should_fail"].As()) { LOG_WARNING() << "Doing Rollback instead of commit " "due to Testpoint response"; - ydb_session_.RollbackTransaction(tx_id_); + ydb_tx_.Rollback(); throw TransactionForceRollback(); } } @@ -93,7 +91,7 @@ void Transaction::Commit(OperationSettings settings) { auto error_guard = ErrorGuard(); impl::GetFutureValueChecked( - ydb_session_.CommitTransaction(tx_id_, commit_settings), "Commit", table_client_.driver_->GetRetryBudget(), context + ydb_tx_.Commit(commit_settings), "Commit", table_client_.driver_->GetRetryBudget(), context ); error_guard.Release(); @@ -114,7 +112,7 @@ void Transaction::Rollback() { [[maybe_unused]] auto error_guard = ErrorGuard(); impl::GetFutureValueChecked( - ydb_session_.RollbackTransaction(tx_id_, rollback_settings), "Rollback", table_client_.driver_->GetRetryBudget(), context + ydb_tx_.Rollback(rollback_settings), "Rollback", table_client_.driver_->GetRetryBudget(), context ); trx_lock_.Unlock(); @@ -123,7 +121,7 @@ void Transaction::Rollback() { } PreparedArgsBuilder Transaction::GetBuilder() const { - return PreparedArgsBuilder(ydb_session_.GetParamsBuilder()); + return PreparedArgsBuilder(ydb_tx_.GetSession().GetParamsBuilder()); } void Transaction::EnsureActive() const { @@ -175,9 +173,9 @@ ExecuteResponse Transaction::Execute( // leaves the transaction active. auto error_guard = ErrorGuard(); - auto execute_fut = ydb_session_.ExecuteQuery( + auto execute_fut = ydb_tx_.GetSession().ExecuteQuery( impl::ToString(query.GetStatementView()), - NYdb::NQuery::TTxControl::Tx(tx_id_), + NYdb::NQuery::TTxControl::Tx(ydb_tx_), std::move(internal_params), exec_settings ); From 1ef6d25ecb76deec5cbc4ec4617d2d61703282cf Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 20 Nov 2025 18:58:24 +0000 Subject: [PATCH 04/10] refactor: extract stats mode conversion to separate function Extract the stats mode conversion logic from Transaction::Execute() into a standalone ConvertStatsMode() helper function in anonymous namespace. This improves code readability and maintainability. Co-authored-by: asmyasnikov <14202262+asmyasnikov@users.noreply.github.com> --- ydb/src/ydb/transaction.cpp | 41 ++++++++++++++++++++++--------------- 1 file changed, 24 insertions(+), 17 deletions(-) diff --git a/ydb/src/ydb/transaction.cpp b/ydb/src/ydb/transaction.cpp index 26b05c5c2a65..cbeb62e2b081 100644 --- a/ydb/src/ydb/transaction.cpp +++ b/ydb/src/ydb/transaction.cpp @@ -17,6 +17,29 @@ USERVER_NAMESPACE_BEGIN namespace ydb { +namespace { + +void ConvertStatsMode(const QuerySettings& query_settings, NYdb::NQuery::TExecuteQuerySettings& exec_settings) { + if (query_settings.collect_query_stats) { + // Convert Table Client stats mode to Query Client stats mode + switch (*query_settings.collect_query_stats) { + case NYdb::NTable::ECollectQueryStatsMode::None: + exec_settings.StatsMode(NYdb::NQuery::EStatsMode::None); + break; + case NYdb::NTable::ECollectQueryStatsMode::Basic: + exec_settings.StatsMode(NYdb::NQuery::EStatsMode::Basic); + break; + case NYdb::NTable::ECollectQueryStatsMode::Full: + exec_settings.StatsMode(NYdb::NQuery::EStatsMode::Full); + break; + case NYdb::NTable::ECollectQueryStatsMode::Profile: + exec_settings.StatsMode(NYdb::NQuery::EStatsMode::Profile); + break; + } + } +} + +} // namespace Transaction::Transaction( TableClient& table_client, @@ -150,23 +173,7 @@ ExecuteResponse Transaction::Execute( if (query_settings.keep_in_query_cache.has_value()) { // Query Client doesn't have KeepInQueryCache, it caches automatically } - if (query_settings.collect_query_stats) { - // Convert Table Client stats mode to Query Client stats mode - switch (*query_settings.collect_query_stats) { - case NYdb::NTable::ECollectQueryStatsMode::None: - exec_settings.StatsMode(NYdb::NQuery::EStatsMode::None); - break; - case NYdb::NTable::ECollectQueryStatsMode::Basic: - exec_settings.StatsMode(NYdb::NQuery::EStatsMode::Basic); - break; - case NYdb::NTable::ECollectQueryStatsMode::Full: - exec_settings.StatsMode(NYdb::NQuery::EStatsMode::Full); - break; - case NYdb::NTable::ECollectQueryStatsMode::Profile: - exec_settings.StatsMode(NYdb::NQuery::EStatsMode::Profile); - break; - } - } + ConvertStatsMode(query_settings, exec_settings); impl::ApplyToRequestSettings(exec_settings, context.settings, context.deadline); // Must go after PrepareExecuteSettings, because an exception from there From 8bf4caff4d4f2e4c01ef4ab1ca07e6bda29bded7 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 20 Nov 2025 20:55:58 +0000 Subject: [PATCH 05/10] refactor: improve ConvertStatsMode function signature Change ConvertStatsMode to take collect_query_stats value as input and return NYdb::NQuery::EStatsMode instead of modifying settings object. This makes the function more functional and easier to understand. Co-authored-by: asmyasnikov <14202262+asmyasnikov@users.noreply.github.com> --- ydb/src/ydb/transaction.cpp | 32 ++++++++++++++------------------ 1 file changed, 14 insertions(+), 18 deletions(-) diff --git a/ydb/src/ydb/transaction.cpp b/ydb/src/ydb/transaction.cpp index cbeb62e2b081..c682863647f2 100644 --- a/ydb/src/ydb/transaction.cpp +++ b/ydb/src/ydb/transaction.cpp @@ -19,23 +19,17 @@ USERVER_NAMESPACE_BEGIN namespace ydb { namespace { -void ConvertStatsMode(const QuerySettings& query_settings, NYdb::NQuery::TExecuteQuerySettings& exec_settings) { - if (query_settings.collect_query_stats) { - // Convert Table Client stats mode to Query Client stats mode - switch (*query_settings.collect_query_stats) { - case NYdb::NTable::ECollectQueryStatsMode::None: - exec_settings.StatsMode(NYdb::NQuery::EStatsMode::None); - break; - case NYdb::NTable::ECollectQueryStatsMode::Basic: - exec_settings.StatsMode(NYdb::NQuery::EStatsMode::Basic); - break; - case NYdb::NTable::ECollectQueryStatsMode::Full: - exec_settings.StatsMode(NYdb::NQuery::EStatsMode::Full); - break; - case NYdb::NTable::ECollectQueryStatsMode::Profile: - exec_settings.StatsMode(NYdb::NQuery::EStatsMode::Profile); - break; - } +NYdb::NQuery::EStatsMode ConvertStatsMode(NYdb::NTable::ECollectQueryStatsMode collect_query_stats) { + // Convert Table Client stats mode to Query Client stats mode + switch (collect_query_stats) { + case NYdb::NTable::ECollectQueryStatsMode::None: + return NYdb::NQuery::EStatsMode::None; + case NYdb::NTable::ECollectQueryStatsMode::Basic: + return NYdb::NQuery::EStatsMode::Basic; + case NYdb::NTable::ECollectQueryStatsMode::Full: + return NYdb::NQuery::EStatsMode::Full; + case NYdb::NTable::ECollectQueryStatsMode::Profile: + return NYdb::NQuery::EStatsMode::Profile; } } @@ -173,7 +167,9 @@ ExecuteResponse Transaction::Execute( if (query_settings.keep_in_query_cache.has_value()) { // Query Client doesn't have KeepInQueryCache, it caches automatically } - ConvertStatsMode(query_settings, exec_settings); + if (query_settings.collect_query_stats) { + exec_settings.StatsMode(ConvertStatsMode(*query_settings.collect_query_stats)); + } impl::ApplyToRequestSettings(exec_settings, context.settings, context.deadline); // Must go after PrepareExecuteSettings, because an exception from there From 0452c8a02dff9b8d9ca1dd0d9f769355674f3bdd Mon Sep 17 00:00:00 2001 From: Aleksey Myasnikov Date: Fri, 21 Nov 2025 12:06:41 +0300 Subject: [PATCH 06/10] Update ydb/include/userver/ydb/transaction.hpp --- ydb/include/userver/ydb/transaction.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ydb/include/userver/ydb/transaction.hpp b/ydb/include/userver/ydb/transaction.hpp index f35b23c96c8e..7d28fe3d5750 100644 --- a/ydb/include/userver/ydb/transaction.hpp +++ b/ydb/include/userver/ydb/transaction.hpp @@ -2,7 +2,7 @@ #include -#include +#include #include #include From 6cd3cb9b4a4febe6ddfaf7229b78d5875ce0b9a8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 21 Nov 2025 09:14:50 +0000 Subject: [PATCH 07/10] refactor: migrate ExecuteDataQuery to Query Client API - Fixed include in transaction.hpp to use query/query.h instead of query/client.h - Added ConvertStatsMode helper function to table.cpp - Refactored ExecuteDataQuery to use Query Client (RetryQuery, ExecuteQuery) instead of Table Client (RetryOperation, ExecuteDataQuery) - Convert QuerySettings to Query Client settings similar to Transaction::Execute - Use PrepareQueryTxSettings and NYdb::NQuery types throughout Co-authored-by: asmyasnikov <14202262+asmyasnikov@users.noreply.github.com> --- ydb/include/userver/ydb/transaction.hpp | 2 +- ydb/src/ydb/table.cpp | 35 ++++++++++++++++++++----- 2 files changed, 30 insertions(+), 7 deletions(-) diff --git a/ydb/include/userver/ydb/transaction.hpp b/ydb/include/userver/ydb/transaction.hpp index 7d28fe3d5750..f35b23c96c8e 100644 --- a/ydb/include/userver/ydb/transaction.hpp +++ b/ydb/include/userver/ydb/transaction.hpp @@ -2,7 +2,7 @@ #include -#include +#include #include #include diff --git a/ydb/src/ydb/table.cpp b/ydb/src/ydb/table.cpp index aca1b3ded06a..9df95e7ba470 100644 --- a/ydb/src/ydb/table.cpp +++ b/ydb/src/ydb/table.cpp @@ -46,6 +46,20 @@ NYdb::NQuery::TTxSettings PrepareQueryTxSettings(const OperationSettings& settin } } +NYdb::NQuery::EStatsMode ConvertStatsMode(NYdb::NTable::ECollectQueryStatsMode collect_query_stats) { + // Convert Table Client stats mode to Query Client stats mode + switch (collect_query_stats) { + case NYdb::NTable::ECollectQueryStatsMode::None: + return NYdb::NQuery::EStatsMode::None; + case NYdb::NTable::ECollectQueryStatsMode::Basic: + return NYdb::NQuery::EStatsMode::Basic; + case NYdb::NTable::ECollectQueryStatsMode::Full: + return NYdb::NQuery::EStatsMode::Full; + case NYdb::NTable::ECollectQueryStatsMode::Profile: + return NYdb::NQuery::EStatsMode::Profile; + } +} + } // namespace TableClient::TableClient( @@ -357,17 +371,26 @@ ExecuteResponse TableClient::ExecuteDataQuery( ) { impl::RequestContext context{*this, query, std::move(settings)}; - auto future = impl::RetryOperation( + // Convert QuerySettings to Query Client settings + NYdb::NQuery::TExecuteQuerySettings exec_settings; + if (query_settings.keep_in_query_cache.has_value()) { + // Query Client doesn't have KeepInQueryCache, it caches automatically + } + if (query_settings.collect_query_stats) { + exec_settings.StatsMode(ConvertStatsMode(*query_settings.collect_query_stats)); + } + + auto future = impl::RetryQuery( context, [query, params = std::move(builder).Build(), - exec_settings = ToExecQuerySettings(query_settings), + exec_settings = std::move(exec_settings), settings = context.settings, - deadline = context.deadline](NYdb::NTable::TSession session) mutable { + deadline = context.deadline](NYdb::NQuery::TSession session) mutable { impl::ApplyToRequestSettings(exec_settings, settings, deadline); - const auto tx_settings = PrepareTxSettings(settings); - const auto tx = NYdb::NTable::TTxControl::BeginTx(tx_settings).CommitTx(); - return session.ExecuteDataQuery(impl::ToString(query.GetStatementView()), tx, params, exec_settings); + const auto tx_settings = PrepareQueryTxSettings(settings); + const auto tx = NYdb::NQuery::TTxControl::BeginTx(tx_settings).CommitTx(); + return session.ExecuteQuery(impl::ToString(query.GetStatementView()), tx, params, exec_settings); } ); From 5d8960174fdcc7033def9c1fa8fc0878eeb5abb2 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 21 Nov 2025 09:39:48 +0000 Subject: [PATCH 08/10] refactor: consolidate ConvertStatsMode into single implementation Moved ConvertStatsMode from duplicate implementations in transaction.cpp and table.cpp to a single shared implementation in impl/operation_settings. Both files now use impl::ConvertStatsMode() to avoid code duplication. Co-authored-by: asmyasnikov <14202262+asmyasnikov@users.noreply.github.com> --- ydb/src/ydb/impl/operation_settings.cpp | 14 ++++++++++++++ ydb/src/ydb/impl/operation_settings.hpp | 4 ++++ ydb/src/ydb/table.cpp | 16 +--------------- ydb/src/ydb/transaction.cpp | 19 +------------------ 4 files changed, 20 insertions(+), 33 deletions(-) diff --git a/ydb/src/ydb/impl/operation_settings.cpp b/ydb/src/ydb/impl/operation_settings.cpp index 14771eaadbed..81673d640c72 100644 --- a/ydb/src/ydb/impl/operation_settings.cpp +++ b/ydb/src/ydb/impl/operation_settings.cpp @@ -27,6 +27,20 @@ std::chrono::milliseconds GetBoundTimeout(std::chrono::milliseconds timeout, eng return (std::chrono::milliseconds::zero() < timeout) ? std::min(timeout, max_timeout) : max_timeout; } +NYdb::NQuery::EStatsMode ConvertStatsMode(NYdb::NTable::ECollectQueryStatsMode collect_query_stats) { + // Convert Table Client stats mode to Query Client stats mode + switch (collect_query_stats) { + case NYdb::NTable::ECollectQueryStatsMode::None: + return NYdb::NQuery::EStatsMode::None; + case NYdb::NTable::ECollectQueryStatsMode::Basic: + return NYdb::NQuery::EStatsMode::Basic; + case NYdb::NTable::ECollectQueryStatsMode::Full: + return NYdb::NQuery::EStatsMode::Full; + case NYdb::NTable::ECollectQueryStatsMode::Profile: + return NYdb::NQuery::EStatsMode::Profile; + } +} + } // namespace ydb::impl USERVER_NAMESPACE_END diff --git a/ydb/src/ydb/impl/operation_settings.hpp b/ydb/src/ydb/impl/operation_settings.hpp index d3cd625cb627..d115d919bad0 100644 --- a/ydb/src/ydb/impl/operation_settings.hpp +++ b/ydb/src/ydb/impl/operation_settings.hpp @@ -2,7 +2,9 @@ #include +#include #include +#include #include #include @@ -14,6 +16,8 @@ USERVER_NAMESPACE_BEGIN namespace ydb::impl { +NYdb::NQuery::EStatsMode ConvertStatsMode(NYdb::NTable::ECollectQueryStatsMode collect_query_stats); + std::chrono::milliseconds GetBoundTimeout(std::chrono::milliseconds timeout, engine::Deadline deadline); template diff --git a/ydb/src/ydb/table.cpp b/ydb/src/ydb/table.cpp index 9df95e7ba470..425664c6e768 100644 --- a/ydb/src/ydb/table.cpp +++ b/ydb/src/ydb/table.cpp @@ -46,20 +46,6 @@ NYdb::NQuery::TTxSettings PrepareQueryTxSettings(const OperationSettings& settin } } -NYdb::NQuery::EStatsMode ConvertStatsMode(NYdb::NTable::ECollectQueryStatsMode collect_query_stats) { - // Convert Table Client stats mode to Query Client stats mode - switch (collect_query_stats) { - case NYdb::NTable::ECollectQueryStatsMode::None: - return NYdb::NQuery::EStatsMode::None; - case NYdb::NTable::ECollectQueryStatsMode::Basic: - return NYdb::NQuery::EStatsMode::Basic; - case NYdb::NTable::ECollectQueryStatsMode::Full: - return NYdb::NQuery::EStatsMode::Full; - case NYdb::NTable::ECollectQueryStatsMode::Profile: - return NYdb::NQuery::EStatsMode::Profile; - } -} - } // namespace TableClient::TableClient( @@ -377,7 +363,7 @@ ExecuteResponse TableClient::ExecuteDataQuery( // Query Client doesn't have KeepInQueryCache, it caches automatically } if (query_settings.collect_query_stats) { - exec_settings.StatsMode(ConvertStatsMode(*query_settings.collect_query_stats)); + exec_settings.StatsMode(impl::ConvertStatsMode(*query_settings.collect_query_stats)); } auto future = impl::RetryQuery( diff --git a/ydb/src/ydb/transaction.cpp b/ydb/src/ydb/transaction.cpp index c682863647f2..c17c1bcca59a 100644 --- a/ydb/src/ydb/transaction.cpp +++ b/ydb/src/ydb/transaction.cpp @@ -17,23 +17,6 @@ USERVER_NAMESPACE_BEGIN namespace ydb { -namespace { - -NYdb::NQuery::EStatsMode ConvertStatsMode(NYdb::NTable::ECollectQueryStatsMode collect_query_stats) { - // Convert Table Client stats mode to Query Client stats mode - switch (collect_query_stats) { - case NYdb::NTable::ECollectQueryStatsMode::None: - return NYdb::NQuery::EStatsMode::None; - case NYdb::NTable::ECollectQueryStatsMode::Basic: - return NYdb::NQuery::EStatsMode::Basic; - case NYdb::NTable::ECollectQueryStatsMode::Full: - return NYdb::NQuery::EStatsMode::Full; - case NYdb::NTable::ECollectQueryStatsMode::Profile: - return NYdb::NQuery::EStatsMode::Profile; - } -} - -} // namespace Transaction::Transaction( TableClient& table_client, @@ -168,7 +151,7 @@ ExecuteResponse Transaction::Execute( // Query Client doesn't have KeepInQueryCache, it caches automatically } if (query_settings.collect_query_stats) { - exec_settings.StatsMode(ConvertStatsMode(*query_settings.collect_query_stats)); + exec_settings.StatsMode(impl::ConvertStatsMode(*query_settings.collect_query_stats)); } impl::ApplyToRequestSettings(exec_settings, context.settings, context.deadline); From ce7c2174924bb05a69812fd9ff7b4f4a09ad5831 Mon Sep 17 00:00:00 2001 From: Aleksey Myasnikov Date: Fri, 21 Nov 2025 14:29:40 +0300 Subject: [PATCH 09/10] Update ydb/src/ydb/impl/operation_settings.cpp Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- ydb/src/ydb/impl/operation_settings.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/ydb/src/ydb/impl/operation_settings.cpp b/ydb/src/ydb/impl/operation_settings.cpp index 81673d640c72..8c501392c57d 100644 --- a/ydb/src/ydb/impl/operation_settings.cpp +++ b/ydb/src/ydb/impl/operation_settings.cpp @@ -39,6 +39,7 @@ NYdb::NQuery::EStatsMode ConvertStatsMode(NYdb::NTable::ECollectQueryStatsMode c case NYdb::NTable::ECollectQueryStatsMode::Profile: return NYdb::NQuery::EStatsMode::Profile; } + return NYdb::NQuery::EStatsMode::None; // Safe fallback for invalid enum values } } // namespace ydb::impl From 5e9a513f98eff79639eaf506a6ad699f8b9e7b45 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 21 Nov 2025 13:22:01 +0000 Subject: [PATCH 10/10] refactor: convert ToExecQuerySettings to free function for Query Client Replaced TableClient::ToExecQuerySettings (which created Table Client settings) with a free function ToExecuteQuerySettings in anonymous namespace that creates NYdb::NQuery::TExecuteQuerySettings for Query Client API. Updated ExecuteDataQuery to use the new function. This completes the migration from Table Client to Query Client API. Co-authored-by: asmyasnikov <14202262+asmyasnikov@users.noreply.github.com> --- ydb/include/userver/ydb/table.hpp | 2 -- ydb/src/ydb/table.cpp | 31 +++++++++++++------------------ 2 files changed, 13 insertions(+), 20 deletions(-) diff --git a/ydb/include/userver/ydb/table.hpp b/ydb/include/userver/ydb/table.hpp index bf7428013205..3bd866701591 100644 --- a/ydb/include/userver/ydb/table.hpp +++ b/ydb/include/userver/ydb/table.hpp @@ -231,8 +231,6 @@ class TableClient final { void Select1(); - NYdb::NTable::TExecDataQuerySettings ToExecQuerySettings(QuerySettings query_settings) const; - template PreparedArgsBuilder MakeBuilder(Args&&... args); diff --git a/ydb/src/ydb/table.cpp b/ydb/src/ydb/table.cpp index 425664c6e768..26769799d45b 100644 --- a/ydb/src/ydb/table.cpp +++ b/ydb/src/ydb/table.cpp @@ -46,6 +46,18 @@ NYdb::NQuery::TTxSettings PrepareQueryTxSettings(const OperationSettings& settin } } +NYdb::NQuery::TExecuteQuerySettings ToExecuteQuerySettings(QuerySettings query_settings) { + // Convert QuerySettings to Query Client settings + NYdb::NQuery::TExecuteQuerySettings exec_settings; + if (query_settings.keep_in_query_cache.has_value()) { + // Query Client doesn't have KeepInQueryCache, it caches automatically + } + if (query_settings.collect_query_stats) { + exec_settings.StatsMode(impl::ConvertStatsMode(*query_settings.collect_query_stats)); + } + return exec_settings; +} + } // namespace TableClient::TableClient( @@ -357,20 +369,11 @@ ExecuteResponse TableClient::ExecuteDataQuery( ) { impl::RequestContext context{*this, query, std::move(settings)}; - // Convert QuerySettings to Query Client settings - NYdb::NQuery::TExecuteQuerySettings exec_settings; - if (query_settings.keep_in_query_cache.has_value()) { - // Query Client doesn't have KeepInQueryCache, it caches automatically - } - if (query_settings.collect_query_stats) { - exec_settings.StatsMode(impl::ConvertStatsMode(*query_settings.collect_query_stats)); - } - auto future = impl::RetryQuery( context, [query, params = std::move(builder).Build(), - exec_settings = std::move(exec_settings), + exec_settings = ToExecuteQuerySettings(query_settings), settings = context.settings, deadline = context.deadline](NYdb::NQuery::TSession session) mutable { impl::ApplyToRequestSettings(exec_settings, settings, deadline); @@ -437,14 +440,6 @@ void DumpMetric(utils::statistics::Writer& writer, const TableClient& table_clie PreparedArgsBuilder TableClient::GetBuilder() const { return PreparedArgsBuilder(table_client_->GetParamsBuilder()); } -NYdb::NTable::TExecDataQuerySettings TableClient::ToExecQuerySettings(QuerySettings query_settings) const { - NYdb::NTable::TExecDataQuerySettings exec_settings; - exec_settings.KeepInQueryCache(query_settings.keep_in_query_cache.value_or(keep_in_query_cache_)); - if (query_settings.collect_query_stats) { - exec_settings.CollectQueryStats(*query_settings.collect_query_stats); - } - return exec_settings; -} } // namespace ydb USERVER_NAMESPACE_END