userver icon indicating copy to clipboard operation
userver copied to clipboard

feat(ydb): migrate transactions and data queries from Table Client to Query Client API

Open Copilot opened this issue 1 month ago • 14 comments

Migration Complete: Replace Table Service with Query Service for YDB Transactions

✅ All Tasks Completed:

  • [x] Understand the YDB SDK Query Client transaction API
  • [x] Analyze the current transaction implementation
  • [x] Design the migration approach to use Query Client transactions
  • [x] Modify Transaction class to use Query Client types (NYdb::NQuery::TTransaction)
  • [x] Update Transaction constructor
  • [x] Convert Transaction::Execute() to use Query Client ExecuteQuery API
  • [x] Convert Transaction::Commit() to use Query Client methods
  • [x] Convert Transaction::Rollback() to use Query Client methods
  • [x] Update Transaction::GetBuilder() to use Query session
  • [x] Modify TableClient::Begin() to use RetryQuery and Query Client BeginTransaction
  • [x] Add YDB-CPP-SDK::Query library to CMakeLists.txt
  • [x] Implement QuerySettings conversion logic
  • [x] Extract stats mode conversion to separate function (code review feedback)
  • [x] Refine ConvertStatsMode function signature (code review feedback)
  • [x] Fix include in transaction.hpp to use query/query.h (code review feedback)
  • [x] Migrate TableClient::ExecuteDataQuery to Query Client API (code review feedback)
  • [x] Consolidate duplicate ConvertStatsMode implementations (code review feedback)
  • [x] Convert ToExecQuerySettings to free function for Query Client (code review feedback)

📝 Summary of Changes:

1. Transaction Class Header (transaction.hpp)

  • Line 5: Changed include to <ydb-cpp-sdk/client/query/query.h> for transaction types
  • Line 74: Changed constructor parameter from NYdb::NTable::TTransaction to NYdb::NQuery::TTransaction
  • Line 90: Changed member variable from NYdb::NTable::TTransaction ydb_tx_ to NYdb::NQuery::TTransaction ydb_tx_

2. Transaction Implementation (transaction.cpp)

  • Constructor (Lines 22-37): Updated to accept NYdb::NQuery::TTransaction instead of Table transaction
  • Commit() (Lines 70-101): Uses ydb_tx_.Commit() with Query Client settings
  • Rollback() (Lines 103-122): Uses ydb_tx_.Rollback() with Query Client settings
  • Execute() (Lines 138-176): Uses ydb_tx_.GetSession().ExecuteQuery() with Query Client APIs and impl::ConvertStatsMode()
  • GetBuilder() (Lines 124-126): Uses ydb_tx_.GetSession().GetParamsBuilder()

3. TableClient Header (table.hpp)

  • Removed: ToExecQuerySettings() method declaration (was creating Table Client settings)

4. TableClient Implementation (table.cpp)

  • ToExecuteQuerySettings() (Lines 49-59): New free function in anonymous namespace that creates NYdb::NQuery::TExecuteQuerySettings for Query Client
  • Begin() (Lines 297-314): Changed from Table Client to Query Client (RetryQuery, PrepareQueryTxSettings, BeginTransaction)
  • ExecuteDataQuery() (Lines 364-387):
    • Migrated to use Query Client
    • Uses new ToExecuteQuerySettings() free function instead of old member method
    • Uses RetryQuery, ExecuteQuery, and Query Client types throughout
  • Removed: Old ToExecQuerySettings() member method that created Table Client settings

5. Implementation Utilities (impl/operation_settings.hpp, impl/operation_settings.cpp)

  • ConvertStatsMode(): Single shared implementation that converts NYdb::NTable::ECollectQueryStatsMode to NYdb::NQuery::EStatsMode
  • Eliminates code duplication between transaction.cpp and table.cpp

6. Build Configuration (CMakeLists.txt, Line 8)

  • Added YDB-CPP-SDK::Query to YDB_LIBRARIES list

🔑 Key Technical Decisions:

  1. Complete Migration: Both Transaction::Execute() and TableClient::ExecuteDataQuery() now use Query Client
  2. No Code Duplication: Consolidated ConvertStatsMode() into shared impl namespace
  3. Free Function Pattern: Converted ToExecQuerySettings from member method to free function ToExecuteQuerySettings that creates Query Client settings
  4. Transaction Object Model: Query Client provides NYdb::NQuery::TTransaction with similar pattern to Table Client
  5. API Conversion: Consistent pattern for converting QuerySettings across both implementations
  6. Backward Compatibility: All public APIs remain unchanged - only internal implementation uses Query Client

🎯 Benefits:

  • Uses modern Query Client API throughout instead of deprecated Table Client
  • Maintains full backward compatibility with existing code
  • Follows YDB SDK recommended migration path
  • No code duplication - single ConvertStatsMode implementation
  • Clean separation of concerns with free functions for conversions
  • Consistent implementation pattern across Transaction and TableClient
  • No changes required to existing code using these APIs

📚 References:

  • Issue: userver-framework/userver#1061
  • YDB Go SDK example: https://github.com/ydb-platform/ydb-go-sdk/blob/70fd60ba1490c984162574958dd23344cb513f6e/internal/table/session.go#L131-L165
  • YDB Java SDK example: https://github.com/ydb-platform/ydb-java-sdk/blob/3fd06ddcf36ab918eb6a394ee3bbebac6caf15b2/query/src/main/java/tech/ydb/query/impl/TableClientImpl.java#L106-L160
Original prompt

This section details on the original issue you should resolve

<issue_title>Заменить Table Service на Query Service при работе с YDB транзакциями</issue_title> <issue_description>При вызове метода Begin создается объект Transaction, который использует для выполнения запросов в YDB устаревший Table Client API. Вместо этого нужно использовать Query Client (класс TQueryClient в YDB С++ SDK).

Для этого нужно:

  1. Создавать транзакцию внутри объекта Transaction через Query Client.
  2. Использовать Query Client для выполнения запросов в методах Execute внутри объекта Transaction
  3. Сохранить сигнатуру существующих методов интерактивной транзакции

Текущая сигнатура Execute зависит от API для Table Client, нужно будет сделать конвертацию под Query Client. Вот примеры такой конвертации в YDB Go SDK и YDB Java SDK.

API Query Client и Table Client немного отличаются, например ExecuteQuery вместо ExecuteDataQuery и RetryQuery вместо RetryOperation. Есть еще отличия, их лучше самому посмотреть в коде YDB C++ SDK: Table Client API и Query Client API.

Код компоненты userver для работы с YDB находится здесь.

Создание транзакции: include, source Работа с транзакциями: include, source</issue_description>

Comments on the Issue (you are @copilot in this section)

  • Fixes userver-framework/userver#1061

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot avatar Nov 20 '25 17:11 Copilot

@asmyasnikov has triggered the import of a pull request, but it hasn't started. If you are a Yandex employee, you can view the log.

robot-magpie[bot] avatar Nov 20 '25 19:11 robot-magpie[bot]

@copilot давай функция ConvertStatsMode будет принимать значение collect_query_stats и отдавать NYdb::NQuery::EStatsMode именно конвертацию...

This was already implemented in commit 8bf4caf. The ConvertStatsMode function takes NYdb::NTable::ECollectQueryStatsMode as input and returns NYdb::NQuery::EStatsMode - it's a pure enum-to-enum conversion with no extra dependencies or calls, just a simple switch statement.

Copilot avatar Nov 20 '25 22:11 Copilot

@copilot давай переделаем реализацию TableClient::ExecuteDataQuery чтобы она работала на query-клиенте (по аналогии с Transaction::Execute

asmyasnikov avatar Nov 21 '25 09:11 asmyasnikov

@copilot давай переделаем реализацию TableClient::ExecuteDataQuery чтобы она работала на query-клиенте (по аналогии с Transaction::Execute

Done. Refactored ExecuteDataQuery to use Query Client API similar to Transaction::Execute. Now uses RetryQuery, ExecuteQuery, and converts QuerySettings to Query Client settings. (commit 6cd3cb9)

Copilot avatar Nov 21 '25 09:11 Copilot

@copilot в твоем коде копипаста ConvertStatsMode - надо оставить единую реализацию

asmyasnikov avatar Nov 21 '25 09:11 asmyasnikov

@copilot в твоем коде копипаста ConvertStatsMode - надо оставить единую реализацию

Fixed. Moved ConvertStatsMode to a single shared implementation in ydb/src/ydb/impl/operation_settings.cpp. Both transaction.cpp and table.cpp now use impl::ConvertStatsMode(). (commit 5d89601)

Copilot avatar Nov 21 '25 09:11 Copilot

LGTM

kpavlov00 avatar Nov 21 '25 10:11 kpavlov00

@kpavlov00 has triggered the import of a pull request, but it hasn't started. If you are a Yandex employee, you can view the log.

robot-magpie[bot] avatar Nov 21 '25 10:11 robot-magpie[bot]

LGTM

kpavlov00 avatar Nov 21 '25 11:11 kpavlov00

@kpavlov00 has triggered the import of a pull request, but it hasn't started. If you are a Yandex employee, you can view the log.

robot-magpie[bot] avatar Nov 21 '25 11:11 robot-magpie[bot]

LGTM

apolukhin avatar Nov 21 '25 11:11 apolukhin

Many thanks for the PR! @apolukhin is now importing your pull request into our internal upstream repository.

robot-magpie[bot] avatar Nov 21 '25 11:11 robot-magpie[bot]

@copilot давай старый метод ToExecQuerySettings превратим в свободную функцию и переделаем для создания NYdb::NQuery::TExecuteQuerySettings

asmyasnikov avatar Nov 21 '25 13:11 asmyasnikov

@copilot давай старый метод ToExecQuerySettings превратим в свободную функцию и переделаем для создания NYdb::NQuery::TExecuteQuerySett...

Done. Converted ToExecQuerySettings to a free function ToExecuteQuerySettings in anonymous namespace that creates NYdb::NQuery::TExecuteQuerySettings for Query Client. Removed the old member method. (commit 5e9a513)

Copilot avatar Nov 21 '25 13:11 Copilot